[PATCH] D139780: [AMDGPU] Unify divergent nodes if the PostDom tree has one root

Wed Jan 4 07:19:42 PST 2023

arsenm accepted this revision.
arsenm added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp:109
   // This is a cluster of orthogonal Transforms
   AU.addPreservedID(LowerSwitchID);
   FunctionPass::getAnalysisUsage(AU);
----------------
ruiling wrote:
> arsenm wrote:
> > ruiling wrote:
> > > gandhi21299 wrote:
> > > > arsenm wrote:
> > > > > We should have a required LowerSwitchID too
> > > > I will have a seperate patch for that, it seems to be causing difficulties when the pass manager schedules UnifyDivergentExitNodes.
> > > I think for function pass dependency or pass ordering, I still prefer they are managed by compiler developer. If I remember correctly, the new pass manager does not support dependency between function passes?
> > The important part is verification. We shouldn't have arbitrary pass contracts not enforced by a verifier
> I agree that pass contracts or assumption should be enforced by verification. For this specific issue, can we verify within this pass that a terminator should not be SwitchInst?
That's what I was asking for for switch handling (should also worry about indirectbr, caller and invoke)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139780