[PATCH] D109958: [LoopFlatten] Enable it by default

Mon Oct 11 10:31:04 PDT 2021

aeubanks added inline comments.

================
Comment at: llvm/test/CodeGen/AMDGPU/opt-pipeline.ll:164
+; GCN-O1-NEXT:       Flattens loops
+; GCN-O1-NEXT:       Memory SSA
 ; GCN-O1-NEXT:       Loop Pass Manager
----------------
SjoerdMeijer wrote:
> SjoerdMeijer wrote:
> > nikic wrote:
> > > I'm somewhat confused by what is going on here. Why do we now calculate MemorySSA and why does LoopUnroll get split into a separate LPM?
> > I have no idea and accepted this as something the pass manager decided to do.... I am also confused about both things: I have no idea why we need to rerun Memory SSA, and don't see why LoopUnroll is now run separately.  I will look into this, see if I can get any wiser here....
> In `lib/Passes/PassBuilderPipelines.cpp` we have this:
> 
>   if (EnableLoopFlatten)
>     FPM.addPass(createFunctionToLoopPassAdaptor(LoopFlattenPass()));
>   // The loop passes in LPM2 (LoopFullUnrollPass) do not preserve MemorySSA.
>   // *All* loop passes must preserve it, in order to be able to use it.
>   FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM2),
>                                               /*UseMemorySSA=*/false,
>                                               /*UseBlockFrequencyInfo=*/false));
> 
> Since this is talking about MemorySSA this might be related, but there are so many things going on here and I am still looking, so don't know for certain. If e.g. @aeubanks has some tips or suggestions here, I would be happy to receive them. :)
This is a legacy PM test.
I'm not super familiar with the legacy PM, but it's probably something to do with the fact that the legacy loop flatten pass is a function pass and perhaps doesn't preserve some analyses?
Could we perhaps just make this change for the new PM?

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

https://reviews.llvm.org/D109958