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

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added a comment.

In D131150#3706666 <https://reviews.llvm.org/D131150#3706666>, @ruiling wrote:

> Hi @Pierre-vh Thanks for working on this. I have to say that the si-annotate-control-flow was not designed to run separately. The control flow lowering process for AMDGPU was completed by several llvm passes running one by one.  Also as @arsenm said, the control flow lowering process might change in the future, so a llc test is more helpful.
>
> For this specific issue, I think this might share the same root-cause with D131181 <https://reviews.llvm.org/D131181>. I have leaved some comments there. My point is the si-annotate-control-flow assume the CFG of the input function should already being structurized, but this case happened to skip the structurization process because the unify-function-divergent-exit tries to optimize for uniformly reachable Unreachable block. I am not sure whether we have any workload depend on such optimization. Changing the behavior of unify-function-divergent-exit might regress quality of generated code in some cases. But teaching the existing structurizer to work on such case might need some non-trivial effort. Any comment? @arsenm

I can remove the opt line then, that's okay. The test is fine either way (fails before the patch, passes after)

If I understand correctly, you think this patch might make codegen worse in some cases? I am not sure what you mean by `unify-function-divergent-exit` 
Does D131181 <https://reviews.llvm.org/D131181> make this patch obsolete?


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

https://reviews.llvm.org/D131150