[PATCH] Disable passes on optnone functions

Robinson, Paul Paul_Robinson at playstation.sony.com
Mon Feb 3 14:15:20 PST 2014


Revised patch for IR-level passes attached.  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






More information about the llvm-commits mailing list