[PATCH] D89820: [AArch64][GlobalISel] Split post-legalizer combiner to allow for lowering at -O0

Wed Oct 21 20:57:35 PDT 2020

aemerson added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64.h:63
 FunctionPass *createAArch64PostLegalizeCombiner(bool IsOptNone);
+FunctionPass *createAArch64PostLegalizeLowering(bool IsOptNone);
 FunctionPass *createAArch64StackTaggingPass(bool IsOptNone);
----------------
paquette wrote:
> aemerson wrote:
> > Not sure why the create functions use a verb instead of the noun, we should keep it consistent.
> I'm going to guess that someone forgot to type the 's' one day and now we have to live with the typo.
Yeah...I'm gonna need you to go ahead and add the 'r' there.

================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp:503
+  GISelKnownBits *KB = &getAnalysis<GISelKnownBitsAnalysis>().get(MF);
+  MachineDominatorTree *MDT =
+      IsOptNone ? nullptr : &getAnalysis<MachineDominatorTree>();
----------------
paquette wrote:
> aemerson wrote:
> > If this is a lowering pass then we may not need a dominator tree and thus IsOptNone variable plumbing through? If something needs to run only w/ optimizations that can live in the actual combiner pass.
> Hm yeah, good point.
> 
> We might also not need known bits either?
Yeah, KB shouldn't be needed either.

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

https://reviews.llvm.org/D89820