[PATCH] D144162: [AMDGPU] Replace LegacyDA with Uniformity Analysis in AnnotateUniformValues

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:81
 void AMDGPUAnnotateUniformValues::visitBranchInst(BranchInst &I) {
-  if (DA->isUniform(&I))
+  if (!UA->hasDivergentTerminator(*I.getParent()))
     setUniformMetadata(&I);
----------------
sameerds wrote:
> sameerds wrote:
> > I am not sure we should be doing this. Smells like a bug in the UA!
> D144699 allows the original "isUniform()" call to work as expected on a BranchInst.
Did you mean to change this back to `isUniform`? (I don't really mind either way.)


================
Comment at: llvm/test/CodeGen/AMDGPU/branch-uniformity.ll:12
 ; CHECK: s_cmp_{{lg|eq}}_u32 s{{[0-9]+}}, 0
-; CHECK: s_cbranch_scc{{[0-1]}}
+; CHECK: s_cbranch_vccz .LBB{{[0-1_]+}}
 ; CHECK: ; %ENDIF53
----------------
This looks like it might be the same kind of regression as `i1-copy-from-loop.ll`


================
Comment at: llvm/test/CodeGen/AMDGPU/divergent-branch-uniform-condition.ll:46
+; ISA-NEXT:    s_cmp_lt_u32 s10, 32
+; ISA-NEXT:    s_cselect_b64 s[12:13], -1, 0
+; ISA-NEXT:    s_mov_b64 s[8:9], -1
----------------
Same kind of regression here I think (the `s_cselect_b64 -1, 0` is a bad smell).


================
Comment at: llvm/test/CodeGen/AMDGPU/i1-copy-from-loop.ll:28
 ; SI-NEXT:    s_cmp_gt_u32 s14, 3
-; SI-NEXT:    s_cbranch_scc1 .LBB0_1
+; SI-NEXT:    s_cselect_b64 s[12:13], -1, 0
+; SI-NEXT:    s_and_b64 vcc, exec, s[12:13]
----------------
This is a regression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144162