[PATCH] D59778: AMDGPU: Make exec mask optimzations more resistant to block splits

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 2 inline comments as done.
arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp:144
+
+    // TODO: Is this really necessary?
+    if (!MBB->isLayoutSuccessor(Succ))
----------------
arsenm wrote:
> rampitec wrote:
> > Yes, in case block is only preceded bit itself. We do not generate this way, but I can write such IR manually. Anyway layout successor means there will be no branch.
> It doesn't mean there's no branch. It still allows
> 
> S_CBRANCH
> <no S_BRANCH>
> 
> <fallthrough block>.
> 
> If both used unconditional branches it shouldn't be any different
I think what's really necessary here is something like:

end_cf sources are both si_if
First si_if dominates the second, and the second end_cf post dominates the first.

This requires moving the pass before the control flow pseudos are lowered


================
Comment at: test/CodeGen/AMDGPU/collapse-endcf.mir:146
   ; GCN:   [[V_CMP_NE_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_NE_U32_e64 2, [[COPY1]], implicit $exec
-  ; GCN:   [[COPY4:%[0-9]+]]:sreg_64 = COPY $exec, implicit-def $exec
-  ; GCN:   [[S_AND_B64_1:%[0-9]+]]:sreg_64 = S_AND_B64 [[COPY4]], [[V_CMP_NE_U32_e64_]], implicit-def dead $scc
+  ; GCN:   [[S_AND_B64_1:%[0-9]+]]:sreg_64 = S_AND_B64 $exec, [[V_CMP_NE_U32_e64_]], implicit-def dead $scc
   ; GCN:   $exec = S_MOV_B64_term [[S_AND_B64_1]]
----------------
rampitec wrote:
> arsenm wrote:
> > rampitec wrote:
> > > I am not sure autogenerated test really tests anything, as there is no GCN-NEXT. The copy may easily remain and be untested.
> > The important part is the number of s_or_b64
> But how you can be sure about the number if there can be some others in between of the checks?
I think it's pretty unlikely this can break with everything checked. update_mir_test_checks should probably be emitting -NEXT, but that's a bigger problem to work on


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

https://reviews.llvm.org/D59778