[PATCH] D19782: [IPO/GlobalDCE] Convert the pass to use static functions.

[Justin Bogner via llvm-commits]

Mehdi Amini <mehdi.amini at apple.com> writes:
>> On May 2, 2016, at 12:55 PM, Justin Bogner <mail at justinbogner.com> wrote:
>> 
>> Mehdi Amini <mehdi.amini at apple.com> writes:
>>>> On May 2, 2016, at 11:20 AM, Justin Bogner <mail at justinbogner.com> wrote:
>>>> 
>>>> Mehdi AMINI <mehdi.amini at apple.com> writes:
>>>>> Davide Italiano <dccitaliano at gmail.com> writes:
>>>>>> Mehdi AMINI <mehdi.amini at apple.com> writes:
>>>>>>> Can we split what is the "pass" and what is the actual utility? 
>>>>>> 
>>>>>> I think it's possible, but maybe overkill? Probably people with more
>>>>>> experience porting passes can tell what they think.
>>>>> 
>>>>> I think this should be the standard layout, it should be the exception
>>>>> to merge together the utility logic and the pass logic.
>>>> 
>>>> Why? One of the nice things about the new pass manager design is that
>>>> the pass *is* a utility. They're just objects that can do the transforms
>>>> you care about and that happen to have a passmanager-compatible run
>>>> method.
>>> 
>>> It does not "happen to have", it *forces you* to have.  If it just
>>> "happened to have" the right interface, there shouldn't be an include
>>> for the PassManager.
>> 
>> Well, the interface uses the PreservedAnalyses and AnalysisManager
>> types, that are defined in the PassManager include, so it's pretty hard
>> to have the right interface without it...
>
> That's just confirming that the interface does not just "happen to be..."

I don't understand what you're trying to say here.

>>>> I don't see how adding an extra layer of indirection would make things
>>>> better, in the general case.
>>> 
>>> single responsibility principle, in the general case...
>> 
>> I'm not convinced having a single utility that is a pass violates SRP.
>> The class does one thing - the transformation. There may be multiple
>> interfaces in terms of how it's called, but that's a different thing.
>> 
>> OTOH, adding a layer of abstraction for no reason falls afoul of the
>> "You aren't gonna need it" adage. It's building something we don't have
>> a use for.
>> 
>> In some cases breaking these up into multiple pieces will certainly make
>> sense and make for a better design, but for simple passes like this it's
>> a waste of time.
>
> It is exactly the same argument that applied when writing pass in the
> past, with great success as we saw.

This is also confusing. The old style passes were specifically designed
not to be useable as utilities - they were type erased and didn't have
any public interface whatsoever. The new style passes are specifically
designed as utilities. How would making them wrappers around other
basically identical utilities help anyone?

Sorry, I really don't think I'm getting what you're trying to say here.