[PATCH] D94748: [AMDGPU] Tidy up conditional branches from early termination

[Carl Ritson via Phabricator via llvm-commits]

critson added a comment.

In D94748#2507651 <https://reviews.llvm.org/D94748#2507651>, @arsenm wrote:

> I'm not sure this is the right pass for this (I've been hoping to move everything out of this pass eventually)

I agree, I think SIPreEmitPeephole is probably a better place for this.



================
Comment at: llvm/lib/Target/AMDGPU/SIInsertSkips.cpp:228
+  // Can we make branch unconditional?
+  bool ReplaceSuccessor = MBB.succ_size() <= 1;
+  if (ReplaceSuccessor)
----------------
foad wrote:
> I'm confused by this. Why can't you generate an unconditional branch to the early exit block from a BB that happened to have two or more successors?
I am not sure why I wrote this either.
I retested without it and could not produce any problems.


================
Comment at: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.wqm.demote.ll:11
 ; SI-NEXT:    v_cmp_gt_f32_e32 vcc, 0, v0
-; SI-NEXT:    s_andn2_b64 exec, exec, exec
-; SI-NEXT:    s_cbranch_scc0 BB0_2
+; SI-NEXT:    s_branch BB0_2
 ; SI-NEXT:  ; %bb.1: ; %.entry
----------------
foad wrote:
> The v_cmp is dead, and so is the whole %bb.1 block. Would you need to run another MachineDCE to get rid of them?
> 
> Alternatively could we get rid of dead code at the IR level? Maybe the AMDGPU instCombineIntrinsic could spot that the argument is false and call setDoesNotReturn() on the call?
Adding a late MachineDCE does clean up the v_cmp, but not %bb.1.  I notice that in some of the other examples it returns other unreachable instructions.  It might be interesting to test results of this on a large collection of shaders.

I am not sure if setting "does not return" is valid, as this will only terminate the whole shader if every lane is demoted.  This is true in uniform control flow, but this intrinsic is valid in non-uniform control flow too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94748