[PATCH] Disable passes on optnone functions

[Robinson, Paul]

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