mehdi_amini added inline comments.

================
Comment at: llvm/test/tools/gold/X86/opt-level.ll:53
+  ; CHECK-O1-OLDPM: select
+  ; The new PM does not do as many optimizations at O1
+  ; CHECK-O1-NEWPM: phi
----------------
tejohnson wrote:
> chandlerc wrote:
> > tejohnson wrote:
> > > tejohnson wrote:
> > > > mehdi_amini wrote:
> > > > > This is intended? I'm surprised the two PMs don't have the same list 
> > > > > of passes for a given opt level?
> > > > I'm really not sure. I did compare the post-link LTO pipelines of both 
> > > > PMs at O0/O1/O2 and confirmed that the old PM is doing many more passes 
> > > > than the new PM at O1. Probably a question for @chandlerc ? In any 
> > > > case, I didn't want to address it here since it is orthogonal.
> > > Some more info:
> > > 
> > > This is the regular LTO post-link pipeline, ThinLTO post-link pipeline 
> > > uses essentially the same as a normal opt pipeline so would be consistent 
> > > at -O1.
> > > 
> > > The optimization missing from the new PM regular LTO post link pipeline 
> > > that affects this test is SimplifyCFG. This and a few other optimizations 
> > > are added in the old PM at O1 via 
> > > PassManagerBuilder::addLateLTOOptimizationPasses. Note that 
> > > PassManagerBuilder::addLTOOptimizationPasses does exit early at -O1 
> > > (similar to where we exit early in the new PM for the regular LTO post 
> > > link compilation). But the "late" LTO optimization passes are added 
> > > unconditionally above -O0. Is this correct/desired? There isn't an 
> > > equivalent in the new PM.
> > I don't think it was intentional, but I'm not sure I would directly 
> > replicate it if it requires adding an entirely new customization point. Is 
> > their some simpler way of getting equivalent results at O1?
> Yeah we can presumably just add those optimizations to the -O1 early exit 
> path in PassBuilder::buildLTODefaultPipeline. I can send a patch (but would 
> like to get this one in as it is a bugfix and orthogonal).
(I fully agree it is orthogonal, I just spotted this as a separate bug indeed, 
thanks for fixing!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61022/new/

https://reviews.llvm.org/D61022



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to