[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 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:
>>>>> ================
>>>>> Comment at: include/llvm/Transforms/IPO/GlobalDCE.h:44
>>>>> @@ +43,3 @@
>>>>> +  bool RemoveUnusedGlobalValue(GlobalValue &GV);
>>>>> +};
>>>>> +
>>>>> ----------------
>>>>> 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...

>
>> 
>> 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.