[AMDGPU] Mark all export instructions as DisableWQM

[cwabbott]

Summary

Export instructions read from EXEC to determine what data to write if
the VM bit is on. If we inadvertently left WQM (or in the future, WWM)
on, we'd write pixels that we weren't supposed to.

[michel.daenzer]

This isn't necessary. Exports can't enable pixels that weren't enabled in the first place.

[cwabbott]

In D34847#796305, @michel.daenzer wrote:

This isn't necessary. Exports can't enable pixels that weren't enabled in the first place.

How sure are you? I haven't found anything in the publically available documentation that would support this assertion, and experimentally it seems this isn't true. This patch (combined with D34847) fixes a fragment shader AMD_shader_ballot test I wrote. The test does addInvocationsNonUniform(1) and then compares the result with the number of active threads with ID less than or equal to the current thread in the subgroup. Due to the way addInvocationsNonUniform is implemented, inactive lanes should return the result of the closest active lane with a smaller thread ID, so the inactive lanes should have a result smaller than the test would expect (but that's ok, because they're inactive!). I wrote the test to report the expected and actual value for each thread that failed, so you would expect any inactive lanes that were accidentally written due to WWM staying enabled would result in pixels with the actual smaller than expected, which is exactly what happened without this patch. With this patch, the test was fixed.

[cwabbott]

After some more testing, it seems like my hypothesis about the issue was wrong. I think the real problem was that WWM wasn't being disabled in the face of control flow like I intended it to, and then stuff was getting messed up because inactive lanes were enabled when the code assumed they weren't. At least, that's my theory for now. I'm going to update D34717 to fix that.

lib/Target/AMDGPU/SIInstrInfo.td

class EXP_Helper<bit done, SDPatternOperator node = null_frag> : EXPCommon<
   (outs),
   (ins exp_tgt:$tgt,
        ExpSrc0:$src0, ExpSrc1:$src1, ExpSrc2:$src2, ExpSrc3:$src3,
        exp_vm:$vm, exp_compr:$compr, i8imm:$en),
   "exp$tgt $src0, $src1, $src2, $src3"#!if(done, " done", "")#"$compr$vm",
   [(node (i8 timm:$tgt), (i8 timm:$en),
          f32:$src0, f32:$src1, f32:$src2, f32:$src3,
          (i1 timm:$compr), (i1 timm:$vm))]> {
+  let DisableWQM = 1;
   let AsmMatchConverter = "cvtExp";
 }
 
 // Split EXP instruction into EXP and EXP_DONE so we can set
 // mayLoad for done=1.
 multiclass EXP_m<bit done, SDPatternOperator node> {
   let mayLoad = done in {
     let isPseudo = 1, isCodeGenOnly = 1 in {

test/CodeGen/AMDGPU/wqm.ll

 ; RUN: llc -march=amdgcn -mcpu=verde -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK -check-prefix=SI %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK -check-prefix=VI %s
 
 ; Check that WQM isn't triggered by image load/store intrinsics.
 ;
 ;CHECK-LABEL: {{^}}test1:
 ;CHECK-NOT: s_wqm
 define amdgpu_ps <4 x float> @test1(<8 x i32> inreg %rsrc, <4 x i32> %c) {
 main_body:
   %tex = call <4 x float> @llvm.amdgcn.image.load.v4f32.v4i32.v8i32(<4 x i32> %c, <8 x i32> %rsrc, i32 15, i1 0, i1 0, i1 0, i1 0)
   call void @llvm.amdgcn.image.store.v4f32.v4i32.v8i32(<4 x float> %tex, <4 x i32> %c, <8 x i32> %rsrc, i32 15, i1 0, i1 0, i1 0, i1 0)
   ret <4 x float> %tex
 }
 
-; Check that WQM is triggered by image samples and left untouched for loads...
+; Check that WQM is triggered by image samples and left untouched for loads,
+; but disabled for exports...
 ;
 ;CHECK-LABEL: {{^}}test2:
 ;CHECK-NEXT: ; %main_body
-;CHECK-NEXT: s_wqm_b64 exec, exec
+;CHECK: s_mov_b64 [[ORIG:s\[[0-9]+:[0-9]+\]]], exec
+;CHECK: s_wqm_b64 exec, exec
+;CHECK-SI: buffer_load_dword
+;CHECK-VI: flat_load_dword
+;CHECK: s_and_b64 exec, exec, [[ORIG]]
 ;CHECK-NOT: exec
-define amdgpu_ps void @test2(<8 x i32> inreg %rsrc, <4 x i32> inreg %sampler, float addrspace(1)* inreg %ptr, <4 x float> %c) {
+;CHECK: exp
+define amdgpu_ps void @test2(<8 x i32> inreg %rsrc, <4 x i32> inreg %sampler, float addrspace(1)* inreg %ptr) {
 main_body:
+  %data = load float, float addrspace(1)* %ptr
+  %c = insertelement <4 x float> zeroinitializer, float %data, i32 0
   %c.1 = call <4 x float> @llvm.amdgcn.image.sample.v4f32.v4f32.v8i32(<4 x float> %c, <8 x i32> %rsrc, <4 x i32> %sampler, i32 15, i1 false, i1 false, i1 false, i1 false, i1 false) #0
-  %c.2 = bitcast <4 x float> %c.1 to <4 x i32>
-  %c.3 = extractelement <4 x i32> %c.2, i32 0
-  %gep = getelementptr float, float addrspace(1)* %ptr, i32 %c.3
-  %data = load float, float addrspace(1)* %gep
-  call void @llvm.amdgcn.exp.f32(i32 0, i32 15, float %data, float undef, float undef, float undef, i1 true, i1 true) #1
+  %c.2 = extractelement <4 x float> %c.1, i32 0
+  call void @llvm.amdgcn.exp.f32(i32 0, i32 15, float %c.2, float undef, float undef, float undef, i1 true, i1 true) #1
   ret void
 }
 
-; ... but disabled for stores (and, in this simple case, not re-enabled).
+; ... and for stores (and, in this simple case, not re-enabled).
 ;
 ;CHECK-LABEL: {{^}}test3:
 ;CHECK-NEXT: ; %main_body
 ;CHECK-NEXT: s_mov_b64 [[ORIG:s\[[0-9]+:[0-9]+\]]], exec
 ;CHECK-NEXT: s_wqm_b64 exec, exec
 ;CHECK: s_and_b64 exec, exec, [[ORIG]]
 ;CHECK: image_sample
 ;CHECK: store