[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