[PATCH] Disable passes on optnone functions

Robinson, Paul Paul_Robinson at playstation.sony.com
Wed Feb 5 16:13:58 PST 2014


> -----Original Message-----
> From: Nick Lewycky [mailto:nicholas at mxc.ca]
> Sent: Wednesday, February 05, 2014 12:40 AM
> To: Robinson, Paul
> Cc: reviews+D2369+public+3a7363072c7f178d at llvm-reviews.chandlerc.com;
> chandlerc at gmail.com; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Disable passes on optnone functions
> 
> 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

r200892.  I'll go back to the MachineFunction passes now.
Thanks,
--paulr

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






More information about the llvm-commits mailing list