[PATCH] Disable passes on optnone functions

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


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

> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-optnone-IR-Passes.patch
Type: application/octet-stream
Size: 23738 bytes
Desc: 2-optnone-IR-Passes.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140203/756cd2b0/attachment.obj>


More information about the llvm-commits mailing list