AMDGPU: Fix @llvm.amdgcn.wqm.vote implementation

[cwabbott]

Summary

My understanding, and the current behavior of the backend, is that
booleans lowered to bitmasks have undefined values in bits corresponding
to lanes that aren't active. If we just lower this to a WQM intrinsic,
then if the values for inactive lanes are 1, then this may change the
result if not every thread in a quad is active. Fix this by AND'ing with
EXEC to mask out the garbage lanes.

This fixes some VK conformance tests when making the clustered subgroup
reduce operations use @llvm.amdgcn.wqm.vote. I added an extra test which
currently miscompiles.

One other way to handle this would be to change the way we lower boolean
operations like NOT so that inactive lanes are always 0. So far as I can
see, in terms of code quality, this would only prevent a few transforms,
like S_ORN2_B64 for src0 | ~src1. The main thing would be lowering NOT
to an XOR with EXEC. Otherwise we'd have to do an analysis to prove
that the AND is redundant in order to get rid of these instructions.
Thoughts?

[nhaehnle]

FWIW, the problem is a bit more involved than that. Consider

bool value = ...;
if (divergent condition) {
  use(value);
}

So the undefinedness of inactive bits in this case is not due to suboptimal lowering of NOT, but inherently due to the fact that we're in a different region of control flow.

This needs a proper solution at some point, possibly with some knownbits-like analysis in the backend to check whether bits outside of EXEC are known to be 0, in order to be able to generate optimal or at least better code in all cases.

For now, I think this is a reasonable enough bug fix.

[cwabbott]

Fixed a regression in the llvm.amdgcn.kill tests.

[arsenm]

llvm/lib/Target/AMDGPU/SOPInstructions.td

1039 ↗

(On Diff #187772)

This needs a comment for why the and is needed

lib/Target/AMDGPU/SOPInstructions.td

let Defs = [SCC] in {
   def S_NOT_B32 : SOP1_32 <"s_not_b32",
     [(set i32:$sdst, (not i32:$src0))]
   >;
 
   def S_NOT_B64 : SOP1_64 <"s_not_b64",
     [(set i64:$sdst, (not i64:$src0))]
   >;
   def S_WQM_B32 : SOP1_32 <"s_wqm_b32">;
-  def S_WQM_B64 : SOP1_64 <"s_wqm_b64",
-    [(set i1:$sdst, (int_amdgcn_wqm_vote i1:$src0))]
-  >;
+  def S_WQM_B64 : SOP1_64 <"s_wqm_b64">;
 } // End Defs = [SCC]
 
 
 def S_BREV_B32 : SOP1_32 <"s_brev_b32",
   [(set i32:$sdst, (bitreverse i32:$src0))]
 >;
 def S_BREV_B64 : SOP1_64 <"s_brev_b64">;
 

 // S_GETREG_B32 Intrinsic Pattern.
 //===----------------------------------------------------------------------===//
 def : GCNPat <
   (int_amdgcn_s_getreg imm:$simm16),
   (S_GETREG_B32 (as_i16imm $simm16))
 >;
 
 //===----------------------------------------------------------------------===//
+// WQM Intrinsic Pattern.
+//===----------------------------------------------------------------------===//
+def : GCNPat<
+  (i1 (int_amdgcn_wqm_vote i1:$src0)),
+  (S_WQM_B64 (S_AND_B64 $src0, (i64 EXEC)))
+>;
+
+
+//===----------------------------------------------------------------------===//
 // SOP1 Patterns
 //===----------------------------------------------------------------------===//
 
 def : GCNPat <
   (i64 (ctpop i64:$src)),
     (i64 (REG_SEQUENCE SReg_64,
      (i32 (COPY_TO_REGCLASS (S_BCNT1_I32_B64 $src), SReg_32)), sub0,
      (S_MOV_B32 (i32 0)), sub1))

test/CodeGen/AMDGPU/llvm.amdgcn.wqm.vote.ll

 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -enable-var-scope -check-prefix=CHECK %s
 
 ;CHECK-LABEL: {{^}}ret:
 ;CHECK: v_cmp_eq_u32_e32 [[CMP:[^,]+]], v0, v1
-;CHECK: s_wqm_b64 [[WQM:[^,]+]], [[CMP]]
+;CHECK: s_and_b64 [[AND:[^,]+]], [[CMP]], exec
+;CHECK: s_wqm_b64 [[WQM:[^,]+]], [[AND]]
 ;CHECK: v_cndmask_b32_e64 v0, 0, 1.0, [[WQM]]
 define amdgpu_ps float @ret(i32 %v0, i32 %v1) #1 {
 main_body:
   %c = icmp eq i32 %v0, %v1
   %w = call i1 @llvm.amdgcn.wqm.vote(i1 %c)
   %r = select i1 %w, float 1.0, float 0.0
   ret float %r
 }

 main_body:
   %w = call i1 @llvm.amdgcn.wqm.vote(i1 false)
   %r = select i1 %w, float 1.0, float 0.0
   ret float %r
 }
 
 ;CHECK-LABEL: {{^}}kill:
 ;CHECK: v_cmp_eq_u32_e32 [[CMP:[^,]+]], v0, v1
-;CHECK: s_wqm_b64 [[WQM:[^,]+]], [[CMP]]
+;CHECK: s_and_b64 [[AND:[^,]+]], [[CMP]], exec
+;CHECK: s_wqm_b64 [[WQM:[^,]+]], [[AND]]
 ;CHECK: s_and_b64 exec, exec, [[WQM]]
 ;CHECK: s_endpgm
 define amdgpu_ps void @kill(i32 %v0, i32 %v1) #1 {
 main_body:
   %c = icmp eq i32 %v0, %v1
   %w = call i1 @llvm.amdgcn.wqm.vote(i1 %c)
   call void @llvm.amdgcn.kill(i1 %w)
   ret void
 }
 
+;CHECK-LABEL: {{^}}phi_not:
+;CHECK: s_xor_b64 [[NOT:[^,]+]], {{[^,]+}}, -1
+;CHECK-NEXT: s_and_b64 [[AND:[^,]+]], [[NOT]], exec
+;CHECK-NEXT: s_wqm_b64 [[WQM:[^,]+]], [[AND]]
+;CHECK-NEXT: v_cndmask_b32_e64 v0, 0, 1.0, [[WQM]]
+define amdgpu_ps float @phi_not(i32 %v0, i32 %v1, i32 %v2) {
+  %cc = icmp eq i32 %v0, 0
+
+  br i1 %cc, label %if, label %else
+
+  if:
+  %tmp1 = icmp ne i32 %v1, 1
+  br label %endif
+
+  else:
+  %tmp2 = icmp eq i32 %v2, 2
+  br label %endif
+
+  endif:
+  %sel = phi i1 [ %tmp1, %if ], [ %tmp2, %else ]
+  %not = xor i1 %sel, true
+  %w = call i1 @llvm.amdgcn.wqm.vote(i1 %not)
+  %r = select i1 %w, float 1.0, float 0.0
+  ret float %r
+}
+
 declare void @llvm.amdgcn.kill(i1) #1
 declare i1 @llvm.amdgcn.wqm.vote(i1)
 
 attributes #1 = { nounwind }