[PATCH] D75267: [AMDGPU] improve fragile test for divergent branches

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds marked an inline comment as done.
sameerds added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/divergent-branch-uniform-condition.ll:8
+; not its condition, so that branch is correctly emitted as divergent.
 
 target triple = "amdgcn-mesa-mesa3d"
----------------
arsenm wrote:
> sameerds wrote:
> > arsenm wrote:
> > > I think this should include ISA generated checks, and we should probably just generate checks for all control flow tests 
> > I am not sure I understand what you mean. Do you want ISA checks in addition to the checks that I am proposing? Also, what is the set of "all control flow tests"?
> Yes. I mean in general we should just generate checks for any control flow test, which this is one of
Yeah, I see the point in general. But the problem with this test is that there is no line in the original input that explicitly captures what is being tested. (See my inline comment elsewhere about the block Flow2.) The new version is a dump of the LLVM IR after structurizer to actually show the branch being tested. Using this version to generate ISA would pass it through the structurizer yet again, which would need a very strong promise on the structurizer being idempotent. It's also possible that something else in the whole codegen flow might make the output insensitive to whether this specific divergent branch is correctly recognized.

Essentially, there are just too many things happening from input to output. As it stands, the new version of the test makes it a proper "unit test" that explicitly shows what is being tested. It's not the code generation that is being examined here, but the handling of divergent branches by specific LLVM IR passes before code generation. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75267