[AMDGPU] Remove IR SpeculativeExecution pass from codegen pipeline

[foad]

Summary

This pass seems to have very little effect because all it does is hoist
some instructions, but it is followed later in the codegen pipeline by
the IR CodeSinking pass which does the opposite.

[foad]

llvm/test/CodeGen/AMDGPU/select-opt.ll

144

In this function SpeculativeExecution hoisted %cmp2 into %if0, which got in the way of CSE removing %cmp2 altogether. With this patch the resulting code is much simpler.

[arsenm]

I doubt we actually want to remove this

[foad]

In D130258#3668409, @arsenm wrote:

I doubt we actually want to remove this

Can you explain why it was added (D20304 does not say) or what it does that does not get undone by code sinking?

[arsenm]

In D130258#3668442, @foad wrote:

In D130258#3668409, @arsenm wrote:

I doubt we actually want to remove this

Can you explain why it was added (D20304 does not say) or what it does that does not get undone by code sinking?

I remember sorely needing this for a long time, and it was added specifically for GPUs. I think we would need compelling benchmarks that it doesn't do anything. It's possible SimplifyCFG has gotten more aggressive since this was added?

[foad]

I've looked at the effect of this patch on our corpus of 10000 graphics shaders (compiled for gfx1030 using a frontend that includes CodeSinking in the IR optimization pipeline).

Out of a total of ~8 million instructions, the instruction frequency deltas look like this:

-89	2935	2846	v_lshl_add_u32
-9	3447	3438	v_mad_u64_u32
-6	59035	59029	s_load_dwordx4
-4	115017	115013	s_clause
-3	56723	56720	s_load_dwordx8
-3	61717	61714	s_mov_b64
-2	1332	1330	v_cmpx_gt_u32
-1	2609	2608	v_cmpx_eq_u32
1	11908	11909	v_cmp_eq_u32
1	257	258	v_ceil_f32
1	5859	5860	v_or_b32
2	1468	1470	v_and_or_b32
2	25853	25855	v_and_b32
2	2924	2926	v_cmp_gt_u32
3	15607	15610	s_and_saveexec_b64
3	6897	6900	s_load_dwordx16
4	1211934	1211938	v_mul_f32
5	14578	14583	s_cbranch_vccnz
5	23221	23226	s_branch
5	301760	301765	s_waitcnt
5	34280	34285	s_and_b64
5	9682	9687	v_cmp_ngt_f32
5	98695	98700	s_buffer_load_dword
9	12325	12334	v_mul_lo_u32
24	760	784	v_subrev_nc_u32
29	187496	187525	v_mov_b32
101	19167	19268	v_lshlrev_b32
144	43422	43566	v_add_nc_u32

For each instruction the 3 numbers are: (delta from A to B), (A = # of occurrences before this patch), (B = # of occurrences after this patch).

So there are maybe 100 shift/add pairs that are no longer combined to v_lshl_add_u32 due to ending up in different basic blocks, but I have looked at a couple of examples and I don't there is anything systematically wrong there, it is just bad luck (and globalisel might fix it anyway by being able to pattern match across basic blocks). Apart from that the diffs seem to be way down in the noise.

[nhaehnle]

The test diff looks surprisingly small. Intuitively speaking, the most interesting instructions for speculative execution are loads. You wouldn't necessarily see a change in the instruction frequency there...

[arsenm]

I’d expect increase in select instructions

[foad]

In D130258#3671596, @nhaehnle wrote:

The test diff looks surprisingly small. Intuitively speaking, the most interesting instructions for speculative execution are loads. You wouldn't necessarily see a change in the instruction frequency there...

Maybe I should have said up front that the SpeculativeExecution pass does not actually do any speculative execution! All it does is "hoists instructions to enable speculative execution", and most of that hoisting is undone by the code sinking pass that we run later.

If I remove CodeSinking but don't remove SpeculativeExecution then I see bad effects where we start with:

bb:
  %gep1 = getelementptr inbounds { i8, i32 }, { i8, i32 } addrspace(5)* %arg0, i32 0, i32 1
  %load1 = load volatile i32, i32 addrspace(5)* %gep1, align 4

and then SpeculativeExecution hoists the gep:

  %gep1 = getelementptr inbounds { i8, i32 }, { i8, i32 } addrspace(5)* %arg0, i32 0, i32 1
  br i1 %cmp, label %bb, label %ret
bb:
  %load1 = load volatile i32, i32 addrspace(5)* %gep1, align 4

and then isel can no longer match the base-plus-constant addressing mode for the load.

[foad]

Ping! I think this is close to being a no-brainer. All SpeculativeExecution does is to hoist some instructions, and virtually all of them are later sunk by the IR code sinking pass.

I've looked at the effect of this patch on our corpus of 10000 graphics shaders (compiled for gfx1030 using a frontend that includes CodeSinking in the IR optimization pipeline).

I redid this analysis and the effect is even smaller than I thought. Only 1 of the 10000 pipelines is affected, and the instruction frequency delta (out of ~900 instructions in that pipeline) is:

2 15291 15293 v_mul_lo_u32
-2 6442 6440 v_or_b32
-1 44265 44264 v_add_nc_u32

[nhaehnle]

Makes sense to me.

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

if (getOptLevel() == CodeGenOpt::Aggressive)
     addPass(createGVNPass());
   else
     addPass(createEarlyCSEPass());
 }
 
 void AMDGPUPassConfig::addStraightLineScalarOptimizationPasses() {
   addPass(createLICMPass());
   addPass(createSeparateConstOffsetFromGEPPass());
-  addPass(createSpeculativeExecutionPass());
   // ReassociateGEPs exposes more opportunities for SLSR. See
   // the example in reassociate-geps-and-slsr.ll.
   addPass(createStraightLineStrengthReducePass());
   // SeparateConstOffsetFromGEP and SLSR creates common expressions which GVN or
   // EarlyCSE can reuse.
   addEarlyCSEOrGVNPass();
   // Run NaryReassociate after EarlyCSE/GVN to be more effective.
   addPass(createNaryReassociatePass());

llvm/test/CodeGen/AMDGPU/divergent-branch-uniform-condition.ll

 ; ISA-NEXT:    v_cmp_nlt_f32_e32 vcc, 0, v0
 ; ISA-NEXT:    s_mov_b64 s[0:1], 0
 ; ISA-NEXT:    ; implicit-def: $sgpr4_sgpr5
 ; ISA-NEXT:    ; implicit-def: $sgpr2_sgpr3
 ; ISA-NEXT:    s_branch .LBB0_3
 ; ISA-NEXT:  .LBB0_1: ; %Flow1
 ; ISA-NEXT:    ; in Loop: Header=BB0_3 Depth=1
 ; ISA-NEXT:    s_or_b64 exec, exec, s[6:7]
-; ISA-NEXT:    s_add_i32 s8, s8, 1
 ; ISA-NEXT:    s_mov_b64 s[6:7], 0
 ; ISA-NEXT:  .LBB0_2: ; %Flow
 ; ISA-NEXT:    ; in Loop: Header=BB0_3 Depth=1
 ; ISA-NEXT:    s_and_b64 s[10:11], exec, s[4:5]
 ; ISA-NEXT:    s_or_b64 s[0:1], s[10:11], s[0:1]
 ; ISA-NEXT:    s_andn2_b64 s[2:3], s[2:3], exec
 ; ISA-NEXT:    s_and_b64 s[6:7], s[6:7], exec
 ; ISA-NEXT:    s_or_b64 s[2:3], s[2:3], s[6:7]
 ; ISA-NEXT:    s_andn2_b64 exec, exec, s[0:1]
 ; ISA-NEXT:    s_cbranch_execz .LBB0_6
 ; ISA-NEXT:  .LBB0_3: ; %loop
 ; ISA-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; ISA-NEXT:    s_or_b64 s[4:5], s[4:5], exec
 ; ISA-NEXT:    s_cmp_lt_u32 s8, 32
 ; ISA-NEXT:    s_mov_b64 s[6:7], -1
 ; ISA-NEXT:    s_cbranch_scc0 .LBB0_2
 ; ISA-NEXT:  ; %bb.4: ; %endif1
 ; ISA-NEXT:    ; in Loop: Header=BB0_3 Depth=1
 ; ISA-NEXT:    s_mov_b64 s[4:5], -1
 ; ISA-NEXT:    s_and_saveexec_b64 s[6:7], vcc
 ; ISA-NEXT:    s_cbranch_execz .LBB0_1
 ; ISA-NEXT:  ; %bb.5: ; %endif2
 ; ISA-NEXT:    ; in Loop: Header=BB0_3 Depth=1
+; ISA-NEXT:    s_add_i32 s8, s8, 1
 ; ISA-NEXT:    s_xor_b64 s[4:5], exec, -1
 ; ISA-NEXT:    s_branch .LBB0_1
 ; ISA-NEXT:  .LBB0_6: ; %Flow2
 ; ISA-NEXT:    s_or_b64 exec, exec, s[0:1]
 ; ISA-NEXT:    v_mov_b32_e32 v1, 0
 ; ISA-NEXT:    s_and_saveexec_b64 s[0:1], s[2:3]
 ; ISA-NEXT:  ; %bb.7: ; %if1
 ; ISA-NEXT:    v_sqrt_f32_e32 v1, v0

llvm/test/CodeGen/AMDGPU/llc-pipeline.ll

 ; GCN-O1-OPTS-NEXT:      LCSSA Verifier
 ; GCN-O1-OPTS-NEXT:      Loop-Closed SSA Form Pass
 ; GCN-O1-OPTS-NEXT:      Scalar Evolution Analysis
 ; GCN-O1-OPTS-NEXT:      Lazy Branch Probability Analysis
 ; GCN-O1-OPTS-NEXT:      Lazy Block Frequency Analysis
 ; GCN-O1-OPTS-NEXT:      Loop Pass Manager
 ; GCN-O1-OPTS-NEXT:        Loop Invariant Code Motion
 ; GCN-O1-OPTS-NEXT:      Split GEPs to a variadic base and a constant offset for better CSE
-; GCN-O1-OPTS-NEXT:      Speculatively execute instructions
 ; GCN-O1-OPTS-NEXT:      Scalar Evolution Analysis
 ; GCN-O1-OPTS-NEXT:      Straight line strength reduction
 ; GCN-O1-OPTS-NEXT:      Early CSE
 ; GCN-O1-OPTS-NEXT:      Scalar Evolution Analysis
 ; GCN-O1-OPTS-NEXT:      Nary reassociation
 ; GCN-O1-OPTS-NEXT:      Early CSE
 ; GCN-O1-OPTS-NEXT:      Post-Dominator Tree Construction
 ; GCN-O1-OPTS-NEXT:      Legacy Divergence Analysis

 ; GCN-O2-NEXT:      LCSSA Verifier
 ; GCN-O2-NEXT:      Loop-Closed SSA Form Pass
 ; GCN-O2-NEXT:      Scalar Evolution Analysis
 ; GCN-O2-NEXT:      Lazy Branch Probability Analysis
 ; GCN-O2-NEXT:      Lazy Block Frequency Analysis
 ; GCN-O2-NEXT:      Loop Pass Manager
 ; GCN-O2-NEXT:        Loop Invariant Code Motion
 ; GCN-O2-NEXT:      Split GEPs to a variadic base and a constant offset for better CSE
-; GCN-O2-NEXT:      Speculatively execute instructions
 ; GCN-O2-NEXT:      Scalar Evolution Analysis
 ; GCN-O2-NEXT:      Straight line strength reduction
 ; GCN-O2-NEXT:      Early CSE
 ; GCN-O2-NEXT:      Scalar Evolution Analysis
 ; GCN-O2-NEXT:      Nary reassociation
 ; GCN-O2-NEXT:      Early CSE
 ; GCN-O2-NEXT:      Post-Dominator Tree Construction
 ; GCN-O2-NEXT:      Legacy Divergence Analysis

 ; GCN-O3-NEXT:      LCSSA Verifier
 ; GCN-O3-NEXT:      Loop-Closed SSA Form Pass
 ; GCN-O3-NEXT:      Scalar Evolution Analysis
 ; GCN-O3-NEXT:      Lazy Branch Probability Analysis
 ; GCN-O3-NEXT:      Lazy Block Frequency Analysis
 ; GCN-O3-NEXT:      Loop Pass Manager
 ; GCN-O3-NEXT:        Loop Invariant Code Motion
 ; GCN-O3-NEXT:      Split GEPs to a variadic base and a constant offset for better CSE
-; GCN-O3-NEXT:      Speculatively execute instructions
 ; GCN-O3-NEXT:      Scalar Evolution Analysis
 ; GCN-O3-NEXT:      Straight line strength reduction
 ; GCN-O3-NEXT:      Phi Values Analysis
 ; GCN-O3-NEXT:      Function Alias Analysis Results
 ; GCN-O3-NEXT:      Memory Dependence Analysis
 ; GCN-O3-NEXT:      Optimization Remark Emitter
 ; GCN-O3-NEXT:      Global Value Numbering
 ; GCN-O3-NEXT:      Scalar Evolution Analysis

llvm/test/CodeGen/AMDGPU/mul_int24.ll

 ; GFX9-NEXT:    v_mov_b32_e32 v1, s0
 ; GFX9-NEXT:    buffer_store_dwordx2 v[0:1], off, s[8:11], 0
 ; GFX9-NEXT:    s_endpgm
 ;
 ; EG-LABEL: simplify_i24_crash:
 ; EG:       ; %bb.0: ; %bb
 ; EG-NEXT:    ALU_PUSH_BEFORE 1, @6, KC0[CB0:0-32], KC1[]
 ; EG-NEXT:    JUMP @5 POP:1
-; EG-NEXT:    ALU 10, @8, KC0[CB0:0-32], KC1[]
+; EG-NEXT:    ALU 14, @8, KC0[CB0:0-32], KC1[]
 ; EG-NEXT:    MEM_RAT_CACHELESS STORE_RAW T0.XY, T1.X, 0
 ; EG-NEXT:    POP @5 POP:1
 ; EG-NEXT:    CF_END
 ; EG-NEXT:    ALU clause starting at 6:
 ; EG-NEXT:     SETNE_INT * T0.W, KC0[2].Z, 0.0,
 ; EG-NEXT:     PRED_SETE_INT * ExecMask,PredicateBit (MASKED), PV.W, 0.0,
 ; EG-NEXT:    ALU clause starting at 8:
-; EG-NEXT:     LSHL T0.W, KC0[2].W, literal.x,
-; EG-NEXT:     LSHL * T1.W, KC0[3].Y, literal.x,
+; EG-NEXT:     MOV T0.X, KC0[3].Y,
+; EG-NEXT:     MOV * T1.X, KC0[2].W,
+; EG-NEXT:     LSHL T0.W, PS, literal.x,
+; EG-NEXT:     LSHL * T1.W, PV.X, literal.x,
 ; EG-NEXT:    8(1.121039e-44), 0(0.000000e+00)
 ; EG-NEXT:     ASHR T1.W, PS, literal.x,
 ; EG-NEXT:     ASHR * T0.W, PV.W, literal.x,
 ; EG-NEXT:    8(1.121039e-44), 0(0.000000e+00)
 ; EG-NEXT:     MOV T2.W, KC0[2].Y,
 ; EG-NEXT:     MULLO_INT * T0.X, PS, PV.W,
 ; EG-NEXT:     LSHR T1.X, PV.W, literal.x,
-; EG-NEXT:     MOV * T0.Y, PS,
+; EG-NEXT:     MOV T0.Y, PS,
+; EG-NEXT:     MOV T0.W, KC0[3].X,
+; EG-NEXT:     MOV * T0.W, KC0[3].Z,
 ; EG-NEXT:    2(2.802597e-45), 0(0.000000e+00)
 ;
 ; CM-LABEL: simplify_i24_crash:
 ; CM:       ; %bb.0: ; %bb
 ; CM-NEXT:    ALU_PUSH_BEFORE 1, @6, KC0[CB0:0-32], KC1[]
 ; CM-NEXT:    JUMP @5 POP:1
-; CM-NEXT:    ALU 13, @8, KC0[CB0:0-32], KC1[]
+; CM-NEXT:    ALU 17, @8, KC0[CB0:0-32], KC1[]
 ; CM-NEXT:    MEM_RAT_CACHELESS STORE_DWORD T0, T1.X
 ; CM-NEXT:    POP @5 POP:1
 ; CM-NEXT:    CF_END
 ; CM-NEXT:    ALU clause starting at 6:
 ; CM-NEXT:     SETNE_INT * T0.W, KC0[2].Z, 0.0,
 ; CM-NEXT:     PRED_SETE_INT * ExecMask,PredicateBit (MASKED), PV.W, 0.0,
 ; CM-NEXT:    ALU clause starting at 8:
-; CM-NEXT:     LSHL T0.Z, KC0[2].W, literal.x,
-; CM-NEXT:     LSHL * T0.W, KC0[3].Y, literal.x,
+; CM-NEXT:     MOV * T0.X, KC0[3].Y,
+; CM-NEXT:     MOV * T1.X, KC0[2].W,
+; CM-NEXT:     LSHL T0.Z, PV.X, literal.x,
+; CM-NEXT:     LSHL * T0.W, T0.X, literal.x,
 ; CM-NEXT:    8(1.121039e-44), 0(0.000000e+00)
 ; CM-NEXT:     MOV T0.Y, KC0[2].Y,
 ; CM-NEXT:     ASHR T1.Z, PV.W, literal.x,
 ; CM-NEXT:     ASHR * T0.W, PV.Z, literal.x,
 ; CM-NEXT:    8(1.121039e-44), 0(0.000000e+00)
 ; CM-NEXT:     MULLO_INT T0.X, T0.W, T1.Z,
 ; CM-NEXT:     MULLO_INT T0.Y (MASKED), T0.W, T1.Z,
 ; CM-NEXT:     MULLO_INT T0.Z (MASKED), T0.W, T1.Z,
 ; CM-NEXT:     MULLO_INT * T0.W (MASKED), T0.W, T1.Z,
 ; CM-NEXT:     LSHR T1.X, T0.Y, literal.x,
-; CM-NEXT:     MOV * T0.Y, PV.X,
+; CM-NEXT:     MOV T0.Y, PV.X,
+; CM-NEXT:     MOV T0.Z, KC0[3].X,
+; CM-NEXT:     MOV * T0.W, KC0[3].Z,
 ; CM-NEXT:    2(2.802597e-45), 0(0.000000e+00)
 bb:
   %cmp = icmp eq i32 %arg0, 0
   br i1 %cmp, label %bb11, label %bb7
 
 bb11:
   %tmp14 = shufflevector <2 x i32> %arg1, <2 x i32> undef, <2 x i32> zeroinitializer
   %tmp16 = shufflevector <2 x i32> %arg2, <2 x i32> undef, <2 x i32> zeroinitializer

llvm/test/CodeGen/AMDGPU/select-opt.ll

define amdgpu_kernel void @opt_select_i64_or_cmp_f32(i64 addrspace(1)* %out, float %a, float %b, float %c, i64 %x, i64 %y) #0 {
   %fcmp0 = fcmp one float %a, %b
   %fcmp1 = fcmp one float %a, %c
   %or = or i1 %fcmp0, %fcmp1
   %select = select i1 %or, i64 %x, i64 %y
   store i64 %select, i64 addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}regression:
 ; GCN: v_cmp_neq_f32_e64 s{{\[[0-9]+:[0-9]+\]}}, s{{[0-9]+}}, 1.0
-; GCN: v_cmp_neq_f32_e64 s{{\[[0-9]+:[0-9]+\]}}, s{{[0-9]+}}, 0
-; GCN: v_cmp_eq_f32_e64 s{{\[[0-9]+:[0-9]+\]}}, s{{[0-9]+}}, 0
 
 define amdgpu_kernel void @regression(float addrspace(1)* %out, float %c0, float %c1) #0 {
 entry:
   %cmp0 = fcmp oeq float %c0, 1.0
   br i1 %cmp0, label %if0, label %endif
 
 if0:
   %cmp1 = fcmp oeq float %c1, 0.0