AMDGPU: Remove unnecessary and on conditional branch

[arsenm]

Summary

The comment explaining why this was necessary is incorrect
in its description of v_cmp's behavior for inactive workitems.

[• tstellarAMD]

We should make sure to test this, because I recall there were some regressions fixed by doing this.

[nhaehnle]

I'm pretty sure this can lead to problems with more complicated conditionals, or at least it once could have done so.

The comment was definitely wrong, v_cmp returns a mask that is & EXEC, but the concern I have is when the condition is some boolean combination of conditions instead of coming straight out of v_cmp. For example, we translate a logical negation to simply XOR with ~0 instead of XOR with EXEC (for good reasons, because the condition may have been computed in a different basic block, but still...). In theory negations can always be folded away by a compination of DeMorgan and inverting the v_cmps, but I'm not sure that that always happens.

[arsenm]

I saw no piglit regressions on Hawaii. I don't think I've seen sc add anything like this before

[nhaehnle]

Sorry for the delay. Now that I'm thinking about this more carefully it's probably okay since it's only for uniform control flow. It might randomly fix or break some of those EXEC = 0 cases, but that's not really relevant for this patch.

[arsenm]

r286134

lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp

void AMDGPUDAGToDAGISel::SelectBRCOND(SDNode *N) {
   SDValue Cond = N->getOperand(1);
 
   if (isCBranchSCC(N)) {
     // This brcond will use S_CBRANCH_SCC*, so let tablegen handle it.
     SelectCode(N);
     return;
   }
 
-  // The result of VOPC instructions is or'd against ~EXEC before it is
-  // written to vcc or another SGPR.  This means that the value '1' is always
-  // written to the corresponding bit for results that are masked.  In order
-  // to correctly check against vccz, we need to and VCC with the EXEC
-  // register in order to clear the value from the masked bits.
-
   SDLoc SL(N);
 
-  SDNode *MaskedCond =
-        CurDAG->getMachineNode(AMDGPU::S_AND_B64, SL, MVT::i1,
-                               CurDAG->getRegister(AMDGPU::EXEC, MVT::i1),
-                               Cond);
-  SDValue VCC = CurDAG->getCopyToReg(N->getOperand(0), SL, AMDGPU::VCC,
-                                     SDValue(MaskedCond, 0),
-                                     SDValue()); // Passing SDValue() adds a
-                                                 // glue output.
+  SDValue VCC = CurDAG->getCopyToReg(N->getOperand(0), SL, AMDGPU::VCC, Cond);
   CurDAG->SelectNodeTo(N, AMDGPU::S_CBRANCH_VCCNZ, MVT::Other,
                        N->getOperand(2), // Basic Block
-                       VCC.getValue(0),  // Chain
-                       VCC.getValue(1)); // Glue
+                       VCC.getValue(0));
   return;
 }
 
 // This is here because there isn't a way to use the generated sub0_sub1 as the
 // subreg index to EXTRACT_SUBREG in tablegen.
 void AMDGPUDAGToDAGISel::SelectATOMIC_CMP_SWAP(SDNode *N) {
   MemSDNode *Mem = cast<MemSDNode>(N);
   unsigned AS = Mem->getAddressSpace();

test/CodeGen/AMDGPU/branch-relaxation.ll

 
 bb3:
   store volatile i32 %cnd, i32 addrspace(1)* %arg
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_conditional_min_long_forward_vcnd_branch:
 ; GCN: s_load_dword [[CND:s[0-9]+]]
-; GCN-DAG: v_cmp_eq_f32_e64 [[CMP:s\[[0-9]+:[0-9]+\]]], [[CND]], 0
 ; GCN-DAG: v_mov_b32_e32 [[V_CND:v[0-9]+]], [[CND]]
-; GCN: s_and_b64 vcc, exec, [[CMP]]
-; GCN-NEXT: s_cbranch_vccz [[LONGBB:BB[0-9]+_[0-9]+]]
+; GCN-DAG: v_cmp_eq_f32_e64 vcc, [[CND]], 0
+; GCN: s_cbranch_vccz [[LONGBB:BB[0-9]+_[0-9]+]]
 
 ; GCN-NEXT: [[LONG_JUMP:BB[0-9]+_[0-9]+]]: ; %bb0
 ; GCN-NEXT: s_getpc_b64 vcc
 ; GCN-NEXT: s_add_u32 vcc_lo, vcc_lo, [[ENDBB:BB[0-9]+_[0-9]+]]-([[LONG_JUMP]]+4)
 ; GCN-NEXT: s_addc_u32 vcc_hi, vcc_hi, 0
 ; GCN-NEXT: s_setpc_b64 vcc
 
 ; GCN-NEXT: [[LONGBB]]:

 ; GCN-NEXT: [[LOOP_BODY]]: ; %loop_body
 ; GCN: v_nop_e64
 ; GCN: v_nop_e64
 ; GCN: v_nop_e64
 ; GCN: v_nop_e64
 ; GCN: v_nop_e64
 ; GCN: v_nop_e64
 ; GCN: ;;#ASMEND
-; GCN-NEXT: s_and_b64 vcc, exec, -1{{$}}
+; GCN-NEXT: s_mov_b64 vcc, -1{{$}}
 ; GCN-NEXT: s_cbranch_vccz [[RET]]
 
 ; GCN-NEXT: [[LONGBB:BB[0-9]+_[0-9]+]]: ; %loop_body
 ; GCN-NEXT: ; in Loop: Header=[[LOOP_BODY]] Depth=1
 ; GCN-NEXT: s_getpc_b64 vcc
 ; GCN-NEXT: s_sub_u32 vcc_lo, vcc_lo, ([[LONGBB]]+4)-[[LOOP_BODY]]
 ; GCN-NEXT: s_subb_u32 vcc_hi, vcc_hi, 0
 ; GCN-NEXT: s_setpc_b64 vcc

 ret:
   store volatile i32 7, i32 addrspace(1)* undef
   ret void
 }
 
 ; GCN-LABEL: {{^}}long_branch_hang:
 ; GCN: s_cmp_lt_i32 s{{[0-9]+}}, 6
 ; GCN-NEXT: s_cbranch_scc1 [[LONG_BR_0:BB[0-9]+_[0-9]+]]
-
-; GCN: s_add_u32 vcc_lo, vcc_lo, [[LONG_BR_DEST0:BB[0-9]+_[0-9]+]]-(
-; GCN: s_setpc_b64
+; GCN-NEXT: s_branch  [[SHORTB:BB[0-9]+_[0-9]+]]
 
 ; GCN-NEXT: [[LONG_BR_0]]:
+; GCN: s_add_u32 vcc_lo, vcc_lo, [[LONG_BR_DEST0:BB[0-9]+_[0-9]+]]-(
 ; GCN: s_setpc_b64
 
-; GCN-NEXT: [[LONG_BR_DEST0]]:
+; GCN: [[SHORTB]]:
 ; GCN-DAG: v_cmp_lt_i32
 ; GCN-DAG: v_cmp_gt_i32
 ; GCN: s_cbranch_vccnz
 
 ; GCN: s_setpc_b64
 ; GCN: s_setpc_b64
 
+; GCN: [[LONG_BR_DEST0]]
 ; GCN: s_cmp_eq_u32
 ; GCN-NEXT: s_cbranch_scc0
 ; GCN: s_setpc_b64
 
 ; GCN: s_endpgm
 define amdgpu_kernel void @long_branch_hang(i32 addrspace(1)* nocapture %arg, i32 %arg1, i32 %arg2, i32 %arg3, i32 %arg4, i64 %arg5) #0 {
 bb:
   %tmp = icmp slt i32 %arg2, 9

test/CodeGen/AMDGPU/cf-loop-on-constant.ll

for.body:
   %inc = add i32 %indvar, 1
   br i1 undef, label %for.body, label %for.exit
 }
 
 ; GCN-LABEL: {{^}}loop_arg_0:
 ; GCN: v_and_b32_e32 v{{[0-9]+}}, 1, v{{[0-9]+}}
 ; GCN: v_cmp_eq_u32_e32 vcc, 1,
 
-; GCN: s_and_b64 s{{\[[0-9]+:[0-9]+\]}}, exec, vcc
 ; GCN: [[LOOPBB:BB[0-9]+_[0-9]+]]
 ; GCN: s_cbranch_vccnz [[LOOPBB]]
 ; GCN-NEXT: ; BB#2
 ; GCN-NEXT: s_endpgm
 define void @loop_arg_0(float addrspace(3)* %ptr, i32 %n, i1 %cond) nounwind {
 entry:
   br label %for.body
 

test/CodeGen/AMDGPU/cgp-bitfield-extract.ll

 ; OPT: %1 = lshr i64 %arg1, 30
 ; OPT-NEXT: %val1 = and i64 %1, 127
 
 ; OPT: ret:
 ; OPT: store
 ; OPT: ret
 
 ; GCN-LABEL: {{^}}sink_ubfe_i64_span_midpoint:
+; GCN: s_lshr_b64 s{{\[}}[[LO:[0-9]+]]:{{[0-9]+}}], s{{\[[0-9]+:[0-9]+\]}}, 30
 ; GCN: s_cbranch_vccnz BB3_2
 
-; GCN: s_lshr_b64 s{{\[}}[[LO:[0-9]+]]:{{[0-9]+}}], s{{\[[0-9]+:[0-9]+\]}}, 30
 ; GCN: s_and_b32 s{{[0-9]+}}, s[[LO]], 0xff
 
 ; GCN: BB3_2:
-; GCN: s_lshr_b64 s{{\[}}[[LO:[0-9]+]]:{{[0-9]+}}], s{{\[[0-9]+:[0-9]+\]}}, 30
 ; GCN: s_and_b32 s{{[0-9]+}}, s[[LO]], 0x7f
 
 ; GCN: BB3_3:
 ; GCN: buffer_store_dwordx2
 define void @sink_ubfe_i64_span_midpoint(i64 addrspace(1)* %out, i64 %arg1) #0 {
 entry:
   %shr = lshr i64 %arg1, 30
   br i1 undef, label %bb0, label %bb1

test/CodeGen/AMDGPU/i1-copy-implicit-def.ll

-; RUN: llc -march=amdgcn -mcpu=SI -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
+; RUN: llc -march=amdgcn -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
 
 ; SILowerI1Copies was not handling IMPLICIT_DEF
 ; SI-LABEL: {{^}}br_implicit_def:
 ; SI: BB#0:
-; SI-NEXT: s_and_b64 vcc, exec
 ; SI-NEXT: s_cbranch_vccnz
 define void @br_implicit_def(i32 addrspace(1)* %out, i32 %arg) #0 {
 bb:
   br i1 undef, label %bb1, label %bb2
 
 bb1:
   store volatile i32 123, i32 addrspace(1)* %out
   ret void
 
 bb2:
   ret void
 }
 
 attributes #0 = { nounwind }

test/CodeGen/AMDGPU/si-annotate-cf-noloop.ll

bb5:                                              ; preds = %bb3, %bb1
   unreachable
 }
 
 
 ; OPT-LABEL: @annotate_ret_noloop(
 ; OPT-NOT: call i1 @llvm.amdgcn.loop
 
 ; GCN-LABEL: {{^}}annotate_ret_noloop:
-; GCN: s_and_b64 vcc
 ; GCN: s_cbranch_vccnz
 ; GCN: s_endpgm
 ; GCN: .Lfunc_end1
 define void @annotate_ret_noloop(<4 x float> addrspace(1)* noalias nocapture readonly %arg) #0 {
 bb:
   %tmp = tail call i32 @llvm.amdgcn.workitem.id.x()
   br label %bb1
 

test/CodeGen/AMDGPU/skip-if-dead.ll

 ; CHECK: v_cndmask_b32_e64 [[PHIREG:v[0-9]+]], 0, -1.0,
 ; CHECK: v_cmpx_le_f32_e32 vcc, 0,
 ; CHECK-NEXT: s_cbranch_execnz [[BB4:BB[0-9]+_[0-9]+]]
 
 ; CHECK: exp
 ; CHECK-NEXT: s_endpgm
 
 ; CHECK: [[KILLBB:BB[0-9]+_[0-9]+]]:
-; CHECK: s_and_b64 vcc, exec,
 ; CHECK-NEXT: s_cbranch_vccz [[PHIBB:BB[0-9]+_[0-9]+]]
 
 ; CHECK: [[PHIBB]]:
 ; CHECK: v_cmp_eq_f32_e32 vcc, 0, [[PHIREG]]
-; CHECK: s_and_b64 vcc, exec, vcc
-; CHECK: s_cbranch_vccz [[ENDBB:BB[0-9]+_[0-9]+]]
+; CHECK-NEXT: s_cbranch_vccz [[ENDBB:BB[0-9]+_[0-9]+]]
 
 ; CHECK: ; %bb10
 ; CHECK: v_mov_b32_e32 v{{[0-9]+}}, 9
 ; CHECK: buffer_store_dword
 
 ; CHECK: [[ENDBB]]:
 ; CHECK-NEXT: s_endpgm
 define amdgpu_ps void @phi_use_def_before_kill() #0 {
bb10:
   br label %end
 
 end:
   ret void
 }
 
 ; CHECK-LABEL: {{^}}no_skip_no_successors:
 ; CHECK: v_cmp_nge_f32
-; CHECK: s_and_b64 vcc, exec,
-; CHECK: s_cbranch_vccz [[SKIPKILL:BB[0-9]+_[0-9]+]]
+; CHECK-NEXT: s_cbranch_vccz [[SKIPKILL:BB[0-9]+_[0-9]+]]
 
 ; CHECK: ; %bb6
 ; CHECK: s_mov_b64 exec, 0
 
 ; CHECK: [[SKIPKILL]]:
 ; CHECK: v_cmp_nge_f32
-; CHECK: s_and_b64 vcc, exec, vcc
-; CHECK: s_cbranch_vccz [[UNREACHABLE:BB[0-9]+_[0-9]+]]
+; CHECK-NEXT: s_cbranch_vccz [[UNREACHABLE:BB[0-9]+_[0-9]+]]
 
 ; CHECK: [[UNREACHABLE]]:
 ; CHECK-NEXT: .Lfunc_end{{[0-9]+}}
 define amdgpu_ps void @no_skip_no_successors(float inreg %arg, float inreg %arg1) #0 {
 bb:
   %tmp = fcmp ult float %arg1, 0.000000e+00
   %tmp2 = fcmp ult float %arg, 0x3FCF5C2900000000
   br i1 %tmp, label %bb6, label %bb3

test/CodeGen/AMDGPU/smrd-vccz-bug.ll

 ; RUN: llc -march=amdgcn -mcpu=verde -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=VCCZ-BUG %s
 ; RUN: llc -march=amdgcn -mcpu=bonaire -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=VCCZ-BUG %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=GCN -check-prefix=NOVCCZ-BUG %s
 
 ; GCN-FUNC: {{^}}vccz_workaround:
 ; GCN: s_load_dword s{{[0-9]+}}, s[{{[0-9]+:[0-9]+}}], 0x0
-; GCN: v_cmp_neq_f32_e64 [[MASK:s\[[0-9]+:[0-9]+\]]], s{{[0-9]+}}, 0{{$}}
-; GCN: s_and_b64 vcc, exec, [[MASK]]
+; GCN: v_cmp_neq_f32_e64 vcc, s{{[0-9]+}}, 0{{$}}
 ; GCN: s_waitcnt lgkmcnt(0)
 ; VCCZ-BUG: s_mov_b64 vcc, vcc
 ; NOVCCZ-BUG-NOT: s_mov_b64 vcc, vcc
 ; GCN: s_cbranch_vccnz [[EXIT:[0-9A-Za-z_]+]]
 ; GCN: buffer_store_dword
 ; GCN: [[EXIT]]:
 ; GCN: s_endpgm
 define void @vccz_workaround(i32 addrspace(2)* %in, i32 addrspace(1)* %out, float %cond) {
 entry:
   %cnd = fcmp oeq float 0.0, %cond
   %sgpr = load volatile i32, i32 addrspace(2)* %in
   br i1 %cnd, label %if, label %endif
 
 if:
   store i32 %sgpr, i32 addrspace(1)* %out
   br label %endif
 
 endif:
   ret void
 }
 
 ; GCN-FUNC: {{^}}vccz_noworkaround:
 ; GCN: v_cmp_neq_f32_e32 vcc, 0, v{{[0-9]+}}
-; GCN: s_and_b64 vcc, exec, vcc
 ; GCN: s_cbranch_vccnz [[EXIT:[0-9A-Za-z_]+]]
 ; GCN: buffer_store_dword
 ; GCN: [[EXIT]]:
 ; GCN: s_endpgm
 define void @vccz_noworkaround(float addrspace(1)* %in, float addrspace(1)* %out) {
 entry:
   %vgpr = load volatile float, float addrspace(1)* %in
   %cnd = fcmp oeq float 0.0, %vgpr

test/CodeGen/AMDGPU/uniform-cfg.ll

done:
   store i32 %value, i32 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_if_vcc:
 ; FIXME: We could use _e32 here if we re-used the 0 from [[STORE_VAL]], and
 ; also scheduled the write first.
 ; GCN-DAG: v_cmp_eq_f32_e64 [[COND:vcc|s\[[0-9]+:[0-9]+\]]], s{{[0-9]+}}, 0{{$}}
-; GCN-DAG: s_and_b64 vcc, exec, [[COND]]
 ; GCN-DAG: v_mov_b32_e32 [[STORE_VAL:v[0-9]+]], 0
 ; GCN: s_cbranch_vccnz [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; Fall-through to the else
 ; GCN: v_mov_b32_e32 [[STORE_VAL]], 1
 
 ; GCN: [[IF_LABEL]]:
 ; GCN: buffer_store_dword [[STORE_VAL]]
done:
   store i32 %value, i32 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_if_swap_br_targets_vcc:
 ; FIXME: We could use _e32 here if we re-used the 0 from [[STORE_VAL]], and
 ; also scheduled the write first.
 ; GCN-DAG: v_cmp_neq_f32_e64 [[COND:vcc|s\[[0-9]+:[0-9]+\]]], s{{[0-9]+}}, 0{{$}}
-; GCN-DAG: s_and_b64 vcc, exec, [[COND]]
 ; GCN-DAG: v_mov_b32_e32 [[STORE_VAL:v[0-9]+]], 0
 ; GCN: s_cbranch_vccnz [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; Fall-through to the else
 ; GCN: v_mov_b32_e32 [[STORE_VAL]], 1
 
 ; GCN: [[IF_LABEL]]:
 ; GCN: buffer_store_dword [[STORE_VAL]]

 ENDIF:                                            ; preds = %IF, %main_body
   ret void
 }
 
 ; GCN-LABEL: {{^}}icmp_users_different_blocks:
 ; GCN: s_load_dword [[COND:s[0-9]+]]
 ; GCN: s_cmp_lt_i32 [[COND]], 1
 ; GCN: s_cbranch_scc1 [[EXIT:[A-Za-z0-9_]+]]
-; GCN: v_cmp_gt_i32_e64 [[MASK:s\[[0-9]+:[0-9]+\]]], [[COND]], 0{{$}}
-; GCN: s_and_b64 vcc, exec, [[MASK]]
+; GCN: v_cmp_gt_i32_e64 vcc, [[COND]], 0{{$}}
 ; GCN: s_cbranch_vccnz [[EXIT]]
 ; GCN: buffer_store
 ; GCN: {{^}}[[EXIT]]:
 ; GCN: s_endpgm
 define void @icmp_users_different_blocks(i32 %cond0, i32 %cond1, i32 addrspace(1)* %out) {
 bb:
   %tmp = tail call i32 @llvm.amdgcn.workitem.id.x() #0
   %cmp0 = icmp sgt i32 %cond0, 0
bb9:                                              ; preds = %bb8, %bb4
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_if_scc_i64_eq:
 ; VI-DAG: s_cmp_eq_u64 s{{\[[0-9]+:[0-9]+\]}}, 0
 ; GCN-DAG: v_mov_b32_e32 [[STORE_VAL:v[0-9]+]], 0
 
 ; SI: v_cmp_eq_u64_e64
-; SI: s_and_b64 vcc, exec,
 ; SI: s_cbranch_vccnz [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; VI: s_cbranch_scc1 [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; Fall-through to the else
 ; GCN: v_mov_b32_e32 [[STORE_VAL]], 1
 
 ; GCN: [[IF_LABEL]]:
done:
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_if_scc_i64_ne:
 ; VI-DAG: s_cmp_lg_u64 s{{\[[0-9]+:[0-9]+\]}}, 0
 ; GCN-DAG: v_mov_b32_e32 [[STORE_VAL:v[0-9]+]], 0
 
 ; SI: v_cmp_ne_u64_e64
-; SI: s_and_b64 vcc, exec,
 ; SI: s_cbranch_vccnz [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; VI: s_cbranch_scc1 [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; Fall-through to the else
 ; GCN: v_mov_b32_e32 [[STORE_VAL]], 1
 
 ; GCN: [[IF_LABEL]]:

 done:
   %value = phi i32 [0, %if], [1, %else]
   store i32 %value, i32 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}uniform_if_scc_i64_sgt:
 ; GCN: v_cmp_gt_i64_e64
-; GCN: s_and_b64 vcc, exec,
 ; GCN: s_cbranch_vccnz [[IF_LABEL:[0-9_A-Za-z]+]]
 
 ; Fall-through to the else
 ; GCN: v_mov_b32_e32 [[STORE_VAL]], 1
 
 ; GCN: [[IF_LABEL]]:
 ; GCN: buffer_store_dword [[STORE_VAL]]
 define void @uniform_if_scc_i64_sgt(i64 %cond, i32 addrspace(1)* %out) {

test/CodeGen/AMDGPU/wqm.ll

 ; CHECK-NEXT: ; %entry
 ; CHECK-NEXT: s_mov_b64 [[LIVE:s\[[0-9]+:[0-9]+\]]], exec
 ; CHECK: s_wqm_b64 exec, exec
 ; CHECK: s_and_b64 exec, exec, [[LIVE]]
 ; CHECK: image_store
 ; CHECK: s_wqm_b64 exec, exec
 ; CHECK-DAG: v_mov_b32_e32 [[CTR:v[0-9]+]], 0
 ; CHECK-DAG: v_mov_b32_e32 [[SEVEN:v[0-9]+]], 0x40e00000
-; CHECK: s_branch [[LOOPHDR:BB[0-9]+_[0-9]+]]
 
+; CHECK: [[LOOPHDR:BB[0-9]+_[0-9]+]]: ; %body
 ; CHECK: v_add_f32_e32 [[CTR]], 2.0, [[CTR]]
-; CHECK: [[LOOPHDR]]: ; %loop
 ; CHECK: v_cmp_lt_f32_e32 vcc, [[SEVEN]], [[CTR]]
-; CHECK: s_cbranch_vccz
+; CHECK: s_cbranch_vccz [[LOOPHDR]]
 ; CHECK: ; %break
 
 ; CHECK: ; return
 define amdgpu_ps <4 x float> @test_loop_vcc(<4 x float> %in) nounwind {
 entry:
   call void @llvm.amdgcn.image.store.v4f32.v4i32.v8i32(<4 x float> %in, <4 x i32> undef, <8 x i32> undef, i32 15, i1 0, i1 0, i1 0, i1 0)
   br label %loop