[AMDGPU] Do not ignore exec use where exec is read as data

[rampitec]

Summary

Compares, v_cndmask_b32, and v_readfirstlane_b32 use EXEC
in a way which modifies the result. This implicit EXEC use
shall not be ignored for the purposes of instruction moves.

[vangthao]

LGTM! Thanks!

[foad]

Compares, v_cndmask_b32, and v_readfirstlane_b32 use EXEC in a way which modifies the result.

This doesn't make any sense to me for cndmask. It handles EXEC exactly the same as any other VALU instruction: it writes a result in any lane which is enabled by EXEC, and does nothing for any lane that is disabled.

[rampitec]

In D117814#3260847, @foad wrote:

Compares, v_cndmask_b32, and v_readfirstlane_b32 use EXEC in a way which modifies the result.

This doesn't make any sense to me for cndmask. It handles EXEC exactly the same as any other VALU instruction: it writes a result in any lane which is enabled by EXEC, and does nothing for any lane that is disabled.

It writes a scalar register and content of that register is different with different exec. Same thing as compare. See the bug found by Sebastian here: D116053.

[foad]

In D117814#3261636, @rampitec wrote:

In D117814#3260847, @foad wrote:

Compares, v_cndmask_b32, and v_readfirstlane_b32 use EXEC in a way which modifies the result.

This doesn't make any sense to me for cndmask. It handles EXEC exactly the same as any other VALU instruction: it writes a result in any lane which is enabled by EXEC, and does nothing for any lane that is disabled.

It writes a scalar register and content of that register is different with different exec. Same thing as compare. See the bug found by Sebastian here: D116053.

No, v_cndmask_b32 does not write a scalar register!

[rampitec]

In D117814#3261792, @foad wrote:

In D117814#3261636, @rampitec wrote:

In D117814#3260847, @foad wrote:

Compares, v_cndmask_b32, and v_readfirstlane_b32 use EXEC in a way which modifies the result.

This doesn't make any sense to me for cndmask. It handles EXEC exactly the same as any other VALU instruction: it writes a result in any lane which is enabled by EXEC, and does nothing for any lane that is disabled.

It writes a scalar register and content of that register is different with different exec. Same thing as compare. See the bug found by Sebastian here: D116053.

No, v_cndmask_b32 does not write a scalar register!

Ugh, my bad!

[rampitec]

Closing this because it is landed. Here is patch to remove cndmask: D117909

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (isVOP1(MI) || isVOP2(MI) || isVOP3(MI) || isSDWA(MI) || isSALU(MI)) {
     return !MI.hasImplicitDef() &&
            MI.getNumImplicitOperands() == MI.getDesc().getNumImplicitUses() &&
            !MI.mayRaiseFPException();
   }
 
   return false;
 }
 
+static bool readsExecAsData(const MachineInstr &MI) {
+  if (MI.isCompare())
+    return true;
+
+  switch (MI.getOpcode()) {
+  default:
+    break;
+  case AMDGPU::V_READFIRSTLANE_B32:
+  case AMDGPU::V_CNDMASK_B64_PSEUDO:
+  case AMDGPU::V_CNDMASK_B32_dpp:
+  case AMDGPU::V_CNDMASK_B32_e32:
+  case AMDGPU::V_CNDMASK_B32_e64:
+  case AMDGPU::V_CNDMASK_B32_sdwa:
+    return true;
+  }
+
+  return false;
+}
+
 bool SIInstrInfo::isIgnorableUse(const MachineOperand &MO) const {
   // Any implicit use of exec by VALU is not a real register read.
   return MO.getReg() == AMDGPU::EXEC && MO.isImplicit() &&
-         isVALU(*MO.getParent());
+         isVALU(*MO.getParent()) && !readsExecAsData(*MO.getParent());
 }
 
 bool SIInstrInfo::areLoadsFromSameBasePtr(SDNode *Load0, SDNode *Load1,
                                           int64_t &Offset0,
                                           int64_t &Offset1) const {
   if (!Load0->isMachineOpcode() || !Load1->isMachineOpcode())
     return false;
 

llvm/test/CodeGen/AMDGPU/licm-valu.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -run-pass=machinelicm -verify-machineinstrs -o - %s | FileCheck -check-prefix=GCN %s
+
+---
+name: hoist_move
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: hoist_move
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_MOV_B32_e32_:%[0-9]+]]:vgpr_32 = V_MOV_B32_e32 1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    S_BRANCH %bb.1
+
+  bb.1:
+    %0:vgpr_32 = V_MOV_B32_e32 1, implicit $exec
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_cmp
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_cmp
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    S_BRANCH %bb.1
+
+  bb.1:
+    %0:sreg_64 = V_CMP_EQ_U32_e64 1, 2, implicit $exec
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_readfirstlane
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_readfirstlane
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_READFIRSTLANE_B32_:%[0-9]+]]:sgpr_32 = V_READFIRSTLANE_B32 [[DEF]], implicit $exec
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    %0:vgpr_32 = IMPLICIT_DEF
+    S_BRANCH %bb.1
+
+  bb.1:
+    %1:sgpr_32 = V_READFIRSTLANE_B32 %0:vgpr_32, implicit $exec
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_cndmask_e64
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_cndmask_e64
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CNDMASK_B32_e64_:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_e64 0, [[DEF]], 0, [[DEF]], [[DEF1]], implicit $exec
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    %0:vgpr_32 = IMPLICIT_DEF
+    %1:sreg_64_xexec = IMPLICIT_DEF
+    S_BRANCH %bb.1
+
+  bb.1:
+    %2:vgpr_32 = V_CNDMASK_B32_e64 0, %0, 0, %0, %1, implicit $exec
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_cndmask_e32
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_cndmask_e32
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CNDMASK_B32_e32_:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_e32 [[DEF]], [[DEF]], implicit undef $vcc, implicit $exec
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    %0:vgpr_32 = IMPLICIT_DEF
+    %1:sreg_64_xexec = IMPLICIT_DEF
+    S_BRANCH %bb.1
+
+  bb.1:
+    %2:vgpr_32 = V_CNDMASK_B32_e32 %0, %0, implicit undef $vcc, implicit $exec
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_cndmask_dpp
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_cndmask_dpp
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CNDMASK_B32_dpp:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_dpp [[DEF]], 0, [[DEF]], 0, [[DEF]], 1, 15, 15, 10, implicit $exec, implicit undef $vcc
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    %0:vgpr_32 = IMPLICIT_DEF
+    %1:sreg_64_xexec = IMPLICIT_DEF
+    S_BRANCH %bb.1
+
+  bb.1:
+    %2:vgpr_32 = V_CNDMASK_B32_dpp %0:vgpr_32, 0, %0:vgpr_32, 0, %0:vgpr_32, 1, 15, 15, 10, implicit $exec, implicit undef $vcc
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...
+---
+name: no_hoist_cndmask_sdwa
+tracksRegLiveness: true
+body:             |
+  ; GCN-LABEL: name: no_hoist_cndmask_sdwa
+  ; GCN: bb.0:
+  ; GCN-NEXT:   successors: %bb.1(0x80000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[DEF:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+  ; GCN-NEXT:   [[DEF1:%[0-9]+]]:sreg_64_xexec = IMPLICIT_DEF
+  ; GCN-NEXT:   S_BRANCH %bb.1
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.1:
+  ; GCN-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT:   [[V_CNDMASK_B32_sdwa:%[0-9]+]]:vgpr_32 = V_CNDMASK_B32_sdwa 0, [[DEF]], 0, [[DEF]], 0, 0, 0, 0, 0, implicit $exec, implicit undef $vcc
+  ; GCN-NEXT:   $exec = S_OR_B64 $exec, 1, implicit-def $scc
+  ; GCN-NEXT:   S_CBRANCH_EXECNZ %bb.1, implicit $exec
+  ; GCN-NEXT:   S_BRANCH %bb.2
+  ; GCN-NEXT: {{  $}}
+  ; GCN-NEXT: bb.2:
+  ; GCN-NEXT:   S_ENDPGM 0
+  bb.0:
+    %0:vgpr_32 = IMPLICIT_DEF
+    %1:sreg_64_xexec = IMPLICIT_DEF
+    S_BRANCH %bb.1
+
+  bb.1:
+    %2:vgpr_32 = V_CNDMASK_B32_sdwa 0, %0:vgpr_32, 0, %0:vgpr_32, 0, 0, 0, 0, 0, implicit $exec, implicit undef $vcc
+    $exec = S_OR_B64 $exec, 1, implicit-def $scc
+    S_CBRANCH_EXECNZ %bb.1, implicit $exec
+    S_BRANCH %bb.2
+
+  bb.2:
+    S_ENDPGM 0
+...

llvm/test/CodeGen/AMDGPU/mul24-pass-ordering.ll

bb23:                                             ; preds = %bb23, %bb
   %tmp47 = add i32 %tmp24, %arg2
   br i1 %tmp37, label %bb23, label %.loopexit
 }
 
 define void @lsr_order_mul24_1(i32 %arg, i32 %arg1, i32 %arg2, float addrspace(3)* nocapture %arg3, i32 %arg4, i32 %arg5, i32 %arg6, i32 %arg7, i32 %arg8, i32 %arg9, float addrspace(1)* nocapture readonly %arg10, i32 %arg11, i32 %arg12, i32 %arg13, i32 %arg14, i32 %arg15, i32 %arg16, i1 zeroext %arg17, i1 zeroext %arg18) #0 {
 ; GFX9-LABEL: lsr_order_mul24_1:
 ; GFX9:       ; %bb.0: ; %bb
 ; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT:    v_cmp_lt_u32_e32 vcc, v0, v1
-; GFX9-NEXT:    s_and_saveexec_b64 s[8:9], vcc
+; GFX9-NEXT:    v_and_b32_e32 v5, 1, v18
+; GFX9-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v5
+; GFX9-NEXT:    v_cmp_lt_u32_e64 s[4:5], v0, v1
+; GFX9-NEXT:    s_and_saveexec_b64 s[8:9], s[4:5]
 ; GFX9-NEXT:    s_cbranch_execz .LBB1_3
 ; GFX9-NEXT:  ; %bb.1: ; %bb19
 ; GFX9-NEXT:    v_cvt_f32_u32_e32 v7, v6
-; GFX9-NEXT:    v_and_b32_e32 v5, 0xffffff, v6
-; GFX9-NEXT:    v_and_b32_e32 v8, 1, v18
 ; GFX9-NEXT:    v_add_u32_e32 v4, v4, v0
-; GFX9-NEXT:    v_rcp_iflag_f32_e32 v6, v7
-; GFX9-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v8
-; GFX9-NEXT:    v_lshl_add_u32 v7, v4, 2, v3
+; GFX9-NEXT:    v_and_b32_e32 v5, 0xffffff, v6
+; GFX9-NEXT:    v_lshl_add_u32 v6, v4, 2, v3
+; GFX9-NEXT:    v_rcp_iflag_f32_e32 v7, v7
 ; GFX9-NEXT:    v_lshlrev_b32_e32 v8, 2, v2
 ; GFX9-NEXT:    v_add_u32_e32 v9, v17, v12
 ; GFX9-NEXT:    s_mov_b64 s[10:11], 0
 ; GFX9-NEXT:    v_mov_b32_e32 v4, 0
 ; GFX9-NEXT:    ; implicit-def: $vgpr3
 ; GFX9-NEXT:  .LBB1_2: ; %bb23
 ; GFX9-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; GFX9-NEXT:    v_cvt_f32_u32_e32 v3, v0
 ; GFX9-NEXT:    v_add_u32_e32 v12, v17, v0
 ; GFX9-NEXT:    v_add_u32_e32 v19, v9, v0
 ; GFX9-NEXT:    v_add_u32_e32 v0, v0, v2
-; GFX9-NEXT:    v_madak_f32 v3, v3, v6, 0x3727c5ac
+; GFX9-NEXT:    v_madak_f32 v3, v3, v7, 0x3727c5ac
 ; GFX9-NEXT:    v_cvt_u32_f32_e32 v3, v3
 ; GFX9-NEXT:    v_mul_u32_u24_e32 v18, v3, v5
 ; GFX9-NEXT:    v_add_u32_e32 v3, v3, v16
 ; GFX9-NEXT:    v_cmp_lt_u32_e64 s[4:5], v3, v13
 ; GFX9-NEXT:    v_mul_lo_u32 v3, v3, v15
 ; GFX9-NEXT:    v_sub_u32_e32 v12, v12, v18
 ; GFX9-NEXT:    v_cmp_lt_u32_e64 s[6:7], v12, v14
 ; GFX9-NEXT:    v_sub_u32_e32 v18, v19, v18
 ; GFX9-NEXT:    s_and_b64 s[4:5], s[4:5], s[6:7]
 ; GFX9-NEXT:    v_add_u32_e32 v3, v18, v3
 ; GFX9-NEXT:    s_and_b64 s[4:5], s[4:5], vcc
 ; GFX9-NEXT:    v_cndmask_b32_e64 v3, 0, v3, s[4:5]
 ; GFX9-NEXT:    v_lshlrev_b64 v[18:19], 2, v[3:4]
 ; GFX9-NEXT:    v_add_co_u32_e64 v18, s[6:7], v10, v18
 ; GFX9-NEXT:    v_addc_co_u32_e64 v19, s[6:7], v11, v19, s[6:7]
 ; GFX9-NEXT:    global_load_dword v3, v[18:19], off
 ; GFX9-NEXT:    v_cmp_ge_u32_e64 s[6:7], v0, v1
 ; GFX9-NEXT:    s_or_b64 s[10:11], s[6:7], s[10:11]
 ; GFX9-NEXT:    s_waitcnt vmcnt(0)
 ; GFX9-NEXT:    v_cndmask_b32_e64 v3, 0, v3, s[4:5]
-; GFX9-NEXT:    ds_write_b32 v7, v3
-; GFX9-NEXT:    v_add_u32_e32 v7, v7, v8
+; GFX9-NEXT:    ds_write_b32 v6, v3
+; GFX9-NEXT:    v_add_u32_e32 v6, v6, v8
 ; GFX9-NEXT:    s_andn2_b64 exec, exec, s[10:11]
 ; GFX9-NEXT:    s_cbranch_execnz .LBB1_2
 ; GFX9-NEXT:  .LBB1_3: ; %Flow3
 ; GFX9-NEXT:    s_or_b64 exec, exec, s[8:9]
 ; GFX9-NEXT:    s_waitcnt lgkmcnt(0)
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
 bb:
   %tmp = icmp ult i32 %arg, %arg1