[PATCH] D131150: [AMDGPU][SIAnnotateCF] Support Conditions Using a Common Basic Block

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp:354
+      // then/else block, we'd have pushed that block twice on the stack.
+      while (isTopOfStack(BB))
         Changed |= closeControlFlow(BB);
----------------
Pierre-vh wrote:
> arsenm wrote:
> > arsenm wrote:
> > > Could you construct a situation where the block appears more times, and not consecutively? It may be better to change the stack to a SetVector
> > The answer is probably no since this is supposed to be structurized already
> I managed to create a similarly problematic situation by changing the function below so the second branch uses the common basic block for the "true" instead of the false. We end up with BB22 buried under BB21 and it cant pop it off the stack because of that:
> Not sure how we can approach this? I'll look into it but any tips are appreciated
To be more specific, I wrote two additional tests:
- One has the first branch use the common block for the True, and the second branch uses it for the False
- The second is the opposite.

The first test is completely fine, but the second fails because bb22 gets buried under bb21 as the latter gets visited before it. bb22 gets trapped on the stack, never popped, and the pass fails.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131150