[PATCH] Disable passes on optnone functions

[Nick Lewycky]

Robinson, Paul wrote:
>> -----Original Message-----
>> From: Robinson, Paul
>> Sent: Monday, February 03, 2014 2:15 PM
>> To: Robinson, Paul; reviews+D2369+public+3a7363072c7f178d at llvm-
>> reviews.chandlerc.com; chandlerc at gmail.com
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: RE: [PATCH] Disable passes on optnone functions
>>
>> Revised patch for IR-level passes attached.
>
> Right. NOW they are attached.

This looks fine to me. I say land it and let people continue in 
post-commit review.

Nick

>> I got tired of
>> pasting the debug diagnostic so I factored the real work into
>> the pass base classes.  For function and basic-block passes I
>> can make the debug output appear only once per function; didn't
>> see a good way to do that for loop passes. Not really critical.
>> Debug output does make it pretty easy to test.  I also threw
>> in a check that optnone doesn't suppress anything at -O0.
>>
>> Let's get this done and then I'll take another look at all the
>> machine-function passes.
>> Thanks,
>> --paulr
>>
>>> -----Original Message-----
>>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>>> bounces at cs.uiuc.edu] On Behalf Of Robinson, Paul
>>> Sent: Thursday, January 30, 2014 2:30 PM
>>> To: reviews+D2369+public+3a7363072c7f178d at llvm-reviews.chandlerc.com;
>>> chandlerc at gmail.com
>>> Cc: llvm-commits at cs.uiuc.edu
>>> Subject: RE: [PATCH] Disable passes on optnone functions
>>>
>>>> -----Original Message-----
>>>> From: Nick Lewycky [mailto:nlewycky at google.com]
>>>> Sent: Wednesday, January 29, 2014 4:02 PM
>>>> To: chandlerc at gmail.com; Robinson, Paul
>>>> Cc: llvm-commits at cs.uiuc.edu; silvas at purdue.edu; nlewycky at google.com
>>>> Subject: Re: [PATCH] Disable passes on optnone functions
>>>>
>>>>
>>>>    Please split the patch in two, one for changes to IR-level passes
>>> and
>>>> pass manager (which should land first -- that should help
>> disambiguate
>>>> when there are pieces that both depend on), one for the codegen
>>> changes.
>>>
>>> Okay.  New patches coming, although it may take a little while, see
>>> the comments below re. loop and basic-block testing.
>>>
>>>>
>>>>    I'm unsure about the codegen changes here. You're adding a test at
>>> the
>>>> beginning of "runOnMachineFunction" to each machine-level pass. The
>>>> machine level requires a fixed set of passes to work in a certain
>> way,
>>>> you can't just disable some passes and expect it to work, the way
>> you
>>>> can with the IR-level passes.
>>>
>>> Let me get the revised IR-level patch done, then we can get back to
>> the
>>> machine-level part.
>>>
>>>>
>>>>    The rationale for making the passes individually disable
>> themselves
>>> on
>>>> optnone is because some passes need to ignore optnone -- the always-
>>>> inliner for example.
>>>>
>>>>    The test probably ought to go into test/Feature/optnone.ll ?
>> Running
>>> -
>>>> O1 and -O2, and opt and llc, it's not really testing the
>> functionattrs
>>>> pass.
>>>
>>> Okay, I misunderstood the intent of test/Transforms/FunctionAttrs.
>>>
>>>>
>>>>    I've reviewed everything except for lib/CodeGen/* and consider it
>>> good
>>>> to commit. Sorry for the delays in review!
>>>
>>> Thanks for doing it!  Now my plans for the next social can change
>>> from "bring a laptop and two goons to make somebody review this" to
>>> "lemme buy you a drink."  :-)
>>>
>>>>
>>>>
>>>> ================
>>>> Comment at: lib/IR/LegacyPassManager.cpp:1292
>>>> @@ -1291,1 +1291,3 @@
>>>>       return false;
>>>> +  if (F.hasFnAttribute(Attribute::OptimizeNone)) {
>>>> +    DEBUG(dbgs()<<  "Skipping pass '"<<  getPassName()
>>>> ----------------
>>>> I thought you weren't going to add it to pass managers but to
>>> individual
>>>> passes? This is a pass manager.
>>>>
>>>> ================
>>>> Comment at: lib/Analysis/LoopPass.cpp:180
>>>> @@ -179,1 +179,3 @@
>>>>   bool LPPassManager::runOnFunction(Function&F) {
>>>> +  if (F.hasFnAttribute(Attribute::OptimizeNone)) {
>>>> +    DEBUG(dbgs()<<  "Skipping pass '"<<  getPassName()
>>>> ----------------
>>>> This is a pass manager?
>>>
>>> BBPassManager and LPPassManager are themselves function passes.
>>> I disabled the function passes that are the collection of basic-block
>>> passes and the collection of loop passes, respectively, on the theory
>>> that optnone is a function-level attribute.  However, I can see given
>>> events such as Chandler's recent conversion of a loop pass to a
>>> function pass that disabling the passes individually could be safer.
>>>
>>> I've worked out how to find the function attributes from a basic
>>> block or loop.  However it's really not feasible (AFAICT) to print out
>>> a "skipping" diagnostic once per function, if the passes are disabled
>>> at the loop or basic block level.  That will make testing these things
>>> distinctly more tedious; basically I'll have to take a functional test
>>> for each optimization, slap 'optnone' on it, and verify that the
>>> optimization did NOT happen.  So it may take a little while before I
>>> can turn in revised patches.
>>>
>>>>
>>>>
>>>> http://llvm-reviews.chandlerc.com/D2369
>>>
>>> Thanks,
>>> --paulr
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits