AMDGPU: Push fneg into bitcast of integer select

[arsenm]

Summary

Avoids some regressions in the math libraries in a future
patch.

[Pierre-vh]

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

605–609

[arsenm]

Rebase

[arsenm]

bd1f7c417fc04f93de6b7bbf8740351e58a90613

[dstuttard]

This commit is causing some regression for us in some games.

Here's a reproducer (you get a cannot yet select error):

target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S
32-A5-G1-ni:7"
target triple = "amdgcn--amdpal"

define amdgpu_cs void @_amdgpu_cs_main(float %.i2369) {
.entry:
  %0 = fcmp uge float %.i2369, 0.000000e+00
  %.i2379 = select i1 %0, i32 1, i32 0
  %.i0436 = bitcast i32 %.i2379 to float
  %.i0440 = fneg float %.i0436
  %1 = fcmp uge float %.i0436, 0.000000e+00
  %.i2495 = select i1 %1, i32 %.i2379, i32 0
  %.i0552 = bitcast i32 %.i2495 to float
  %.i0592 = fmul float %.i0440, %.i0552
  %.i0721 = fcmp ogt float %.i0592, 0.000000e+00
  br i1 %.i0721, label %6, label %2

2:                                                ; preds = %.entry
  %3 = call <2 x i32> @llvm.amdgcn.s.buffer.load.v2i32(<4 x i32> zeroinitializer, i32 1, i32 0)
  %4 = shufflevector <2 x i32> %3, <2 x i32> zeroinitializer, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
  %5 = bitcast <4 x i32> %4 to <4 x float>
  %.i0753 = extractelement <4 x float> %5, i64 0
  br label %6

6:                                                ; preds = %2, %.entry
  ret void
}

; Function Attrs: nocallback nofree nosync nounwind willreturn memory(none)
declare <2 x i32> @llvm.amdgcn.s.buffer.load.v2i32(<4 x i32>, i32, i32 immarg) #0

attributes #0 = { nocallback nofree nosync nounwind willreturn memory(none) }

It looks to me that the part that does a "ReplaceAllUsesWith" is causing the problem since it replaces a select returning an i32 with one returning a float, which in this case causes failure in the subsequent bitcast. (See %.i0436 bitcast)

[arsenm]

In D142749#4283463, @dstuttard wrote:

This commit is causing some regression for us in some games.

Here's a reproducer (you get a cannot yet select error):

I have a patch for what I assume is the same problem already

[dstuttard]

In D142749#4283503, @arsenm wrote:

In D142749#4283463, @dstuttard wrote:

This commit is causing some regression for us in some games.

Here's a reproducer (you get a cannot yet select error):

I have a patch for what I assume is the same problem already

Great - are you likely to submit it today, otherwise I'll revert the change locally?

[arsenm]

In D142749#4283549, @dstuttard wrote:

In D142749#4283503, @arsenm wrote:

In D142749#4283463, @dstuttard wrote:

Great - are you likely to submit it today, otherwise I'll revert the change locally?

2fce50e8f583604d49e3bdefde012de244d1e86b is the simple version

[dstuttard]

Thanks - that's fixed the issues I was seeing.

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

 }
 
 static bool fnegFoldsIntoOp(const SDNode *N) {
   unsigned Opc = N->getOpcode();
   if (Opc == ISD::BITCAST) {
     // TODO: Is there a benefit to checking the conditions performFNegCombine
     // does? We don't for the other cases.
     SDValue BCSrc = N->getOperand(0);
-    return BCSrc.getOpcode() == ISD::BUILD_VECTOR &&
-           BCSrc.getNumOperands() == 2 &&
+    if (BCSrc.getOpcode() == ISD::BUILD_VECTOR) {
+      return BCSrc.getNumOperands() == 2 &&
              BCSrc.getOperand(1).getValueSizeInBits() == 32;
     }
 
+    if (BCSrc.getOpcode() == ISD::SELECT && BCSrc.getValueType() == MVT::f32)
+      return true;
+
+    return false;
+  }
+
   return fnegFoldsIntoOpcode(Opc);
 }
 
 /// \p returns true if the operation will definitely need to use a 64-bit
 /// encoding, and thus will use a VOP3 encoding regardless of the source
 /// modifiers.
 LLVM_READONLY
 static bool opMustUseVOP3Encoding(const SDNode *N, MVT VT) {
if (BCSrc.getOpcode() == ISD::BUILD_VECTOR) {
           DAG.getNode(ISD::BUILD_VECTOR, SL, BCSrc.getValueType(), Ops);
       SDValue Result = DAG.getNode(ISD::BITCAST, SL, VT, Build);
 
       if (!N0.hasOneUse())
         DAG.ReplaceAllUsesWith(N0, DAG.getNode(ISD::FNEG, SL, VT, Result));
       return Result;
     }
 
+    if (BCSrc.getOpcode() == ISD::SELECT && VT == MVT::f32) {
+      // fneg (bitcast (f32 (select cond, i32:lhs, i32:rhs))) ->
+      //   select cond, (bitcast i32:lhs to f32), (bitcast i32:rhs to f32)
+      SDValue LHS =
+          DAG.getNode(ISD::BITCAST, SL, MVT::f32, BCSrc.getOperand(1));
+      SDValue RHS =
+          DAG.getNode(ISD::BITCAST, SL, MVT::f32, BCSrc.getOperand(2));
+
+      SDValue NegLHS = DAG.getNode(ISD::FNEG, SL, MVT::f32, LHS);
+      SDValue NegRHS = DAG.getNode(ISD::FNEG, SL, MVT::f32, RHS);
+
+      SDValue NewSelect = DAG.getNode(ISD::SELECT, SL, MVT::f32,
+                                      BCSrc.getOperand(0), NegLHS, NegRHS);
+      if (!BCSrc.hasOneUse())
+        DAG.ReplaceAllUsesWith(BCSrc,
+                               DAG.getNode(ISD::FNEG, SL, VT, NewSelect));
+      return NewSelect;
+    }
+
     return SDValue();
   }
   default:
     return SDValue();
   }
 }
 
 SDValue AMDGPUTargetLowering::performFAbsCombine(SDNode *N,

llvm/test/CodeGen/AMDGPU/fneg-modifier-casting.ll

; GFX11-NEXT:    s_setpc_b64 s[30:31]
   ret double %fneg
 }
 
 define double @fneg_xor_select_f64_multi_user(i1 %cond, double %arg0, double %arg1, ptr addrspace(1) %ptr) {
 ; GFX7-LABEL: fneg_xor_select_f64_multi_user:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_and_b32_e32 v0, 1, v0
-; GFX7-NEXT:    v_mov_b32_e32 v7, v1
 ; GFX7-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v0
-; GFX7-NEXT:    v_cndmask_b32_e32 v1, v4, v2, vcc
-; GFX7-NEXT:    v_cndmask_b32_e32 v0, v3, v7, vcc
+; GFX7-NEXT:    v_cndmask_b32_e64 v2, -v4, -v2, vcc
+; GFX7-NEXT:    v_cndmask_b32_e32 v0, v3, v1, vcc
+; GFX7-NEXT:    v_xor_b32_e32 v1, 0x80000000, v2
 ; GFX7-NEXT:    flat_store_dwordx2 v[5:6], v[0:1]
-; GFX7-NEXT:    v_xor_b32_e32 v1, 0x80000000, v1
+; GFX7-NEXT:    v_mov_b32_e32 v1, v2
 ; GFX7-NEXT:    s_waitcnt vmcnt(0)
 ; GFX7-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX9-LABEL: fneg_xor_select_f64_multi_user:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX9-NEXT:    v_and_b32_e32 v0, 1, v0
-; GFX9-NEXT:    v_mov_b32_e32 v7, v1
 ; GFX9-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v0
-; GFX9-NEXT:    v_cndmask_b32_e32 v1, v4, v2, vcc
-; GFX9-NEXT:    v_cndmask_b32_e32 v0, v3, v7, vcc
+; GFX9-NEXT:    v_cndmask_b32_e64 v2, -v4, -v2, vcc
+; GFX9-NEXT:    v_cndmask_b32_e32 v0, v3, v1, vcc
+; GFX9-NEXT:    v_xor_b32_e32 v1, 0x80000000, v2
 ; GFX9-NEXT:    global_store_dwordx2 v[5:6], v[0:1], off
-; GFX9-NEXT:    v_xor_b32_e32 v1, 0x80000000, v1
+; GFX9-NEXT:    v_mov_b32_e32 v1, v2
 ; GFX9-NEXT:    s_waitcnt vmcnt(0)
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX11-LABEL: fneg_xor_select_f64_multi_user:
 ; GFX11:       ; %bb.0:
 ; GFX11-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX11-NEXT:    s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT:    v_dual_mov_b32 v7, v1 :: v_dual_and_b32 v0, 1, v0
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_2)
+; GFX11-NEXT:    v_and_b32_e32 v0, 1, v0
+; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_2) | instid1(VALU_DEP_2)
 ; GFX11-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 1, v0
-; GFX11-NEXT:    v_dual_cndmask_b32 v1, v4, v2 :: v_dual_cndmask_b32 v0, v3, v7
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1)
-; GFX11-NEXT:    v_xor_b32_e32 v2, 0x80000000, v1
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, -v4, -v2, vcc_lo
+; GFX11-NEXT:    v_cndmask_b32_e32 v0, v3, v1, vcc_lo
+; GFX11-NEXT:    v_xor_b32_e32 v1, 0x80000000, v2
 ; GFX11-NEXT:    global_store_b64 v[5:6], v[0:1], off
 ; GFX11-NEXT:    v_mov_b32_e32 v1, v2
 ; GFX11-NEXT:    s_waitcnt_vscnt null, 0x0
 ; GFX11-NEXT:    s_setpc_b64 s[30:31]
   %select = select i1 %cond, double %arg0, double %arg1
   store double %select, ptr addrspace(1) %ptr
   %fneg = fneg double %select
   ret double %fneg
; GFX11-NEXT:    s_setpc_b64 s[30:31]
   ret double %add
 }
 
 define double @select_fneg_select_fneg_f64(i1 %cond0, i1 %cond1, double %arg0, double %arg1) {
 ; GCN-LABEL: select_fneg_select_fneg_f64:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    v_and_b32_e32 v0, 1, v0
+; GCN-NEXT:    v_and_b32_e32 v1, 1, v1
 ; GCN-NEXT:    v_xor_b32_e32 v3, 0x80000000, v3
 ; GCN-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v0
-; GCN-NEXT:    v_and_b32_e32 v1, 1, v1
 ; GCN-NEXT:    v_cndmask_b32_e32 v0, v2, v4, vcc
-; GCN-NEXT:    v_cndmask_b32_e32 v2, v3, v5, vcc
-; GCN-NEXT:    v_xor_b32_e32 v3, 0x80000000, v2
+; GCN-NEXT:    v_cndmask_b32_e64 v2, -v3, -v5, vcc
 ; GCN-NEXT:    v_cmp_eq_u32_e32 vcc, 1, v1
-; GCN-NEXT:    v_cndmask_b32_e32 v1, v2, v3, vcc
+; GCN-NEXT:    v_cndmask_b32_e64 v1, -v2, v2, vcc
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX11-LABEL: select_fneg_select_fneg_f64:
 ; GFX11:       ; %bb.0:
 ; GFX11-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX11-NEXT:    s_waitcnt_vscnt null, 0x0
 ; GFX11-NEXT:    v_and_b32_e32 v0, 1, v0
 ; GFX11-NEXT:    v_xor_b32_e32 v3, 0x80000000, v3
-; GFX11-NEXT:    v_and_b32_e32 v1, 1, v1
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_3) | instskip(SKIP_1) | instid1(VALU_DEP_4)
+; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(SKIP_1) | instid1(VALU_DEP_3)
 ; GFX11-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 1, v0
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, v2, v4, vcc_lo
-; GFX11-NEXT:    v_cndmask_b32_e32 v2, v3, v5, vcc_lo
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_4) | instskip(NEXT) | instid1(VALU_DEP_2)
+; GFX11-NEXT:    v_dual_cndmask_b32 v0, v2, v4 :: v_dual_and_b32 v1, 1, v1
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, -v3, -v5, vcc_lo
+; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_2) | instskip(NEXT) | instid1(VALU_DEP_2)
 ; GFX11-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 1, v1
-; GFX11-NEXT:    v_xor_b32_e32 v3, 0x80000000, v2
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1)
-; GFX11-NEXT:    v_cndmask_b32_e32 v1, v2, v3, vcc_lo
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, -v2, v2, vcc_lo
 ; GFX11-NEXT:    s_setpc_b64 s[30:31]
   %fneg0 = fneg double %arg0
   %select0 = select i1 %cond0, double %arg1, double %fneg0
   %fneg1 = fneg double %select0
   %select1 = select i1 %cond1, double %fneg1, double %select0
   ret double %select1
 }
 

 
 define double @cospiD_pattern1(i32 %arg, double %arg1, double %arg2) {
 ; GCN-LABEL: cospiD_pattern1:
 ; GCN:       ; %bb.0:
 ; GCN-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GCN-NEXT:    v_and_b32_e32 v5, 1, v0
 ; GCN-NEXT:    v_cmp_eq_u32_e32 vcc, 0, v5
 ; GCN-NEXT:    v_cndmask_b32_e32 v3, v1, v3, vcc
-; GCN-NEXT:    v_cndmask_b32_e32 v1, v2, v4, vcc
-; GCN-NEXT:    v_xor_b32_e32 v2, 0x80000000, v1
+; GCN-NEXT:    v_cndmask_b32_e64 v1, -v2, -v4, vcc
 ; GCN-NEXT:    v_cmp_lt_i32_e32 vcc, 1, v0
-; GCN-NEXT:    v_cndmask_b32_e32 v1, v1, v2, vcc
+; GCN-NEXT:    v_cndmask_b32_e64 v1, -v1, v1, vcc
 ; GCN-NEXT:    v_mov_b32_e32 v0, v3
 ; GCN-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX11-LABEL: cospiD_pattern1:
 ; GFX11:       ; %bb.0:
 ; GFX11-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX11-NEXT:    s_waitcnt_vscnt null, 0x0
 ; GFX11-NEXT:    v_and_b32_e32 v5, 1, v0
 ; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_1) | instskip(SKIP_3) | instid1(VALU_DEP_3)
 ; GFX11-NEXT:    v_cmp_eq_u32_e32 vcc_lo, 0, v5
 ; GFX11-NEXT:    v_cndmask_b32_e32 v3, v1, v3, vcc_lo
-; GFX11-NEXT:    v_cndmask_b32_e32 v1, v2, v4, vcc_lo
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, -v2, -v4, vcc_lo
 ; GFX11-NEXT:    v_cmp_lt_i32_e32 vcc_lo, 1, v0
 ; GFX11-NEXT:    v_mov_b32_e32 v0, v3
-; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_3) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT:    v_xor_b32_e32 v2, 0x80000000, v1
-; GFX11-NEXT:    v_cndmask_b32_e32 v1, v1, v2, vcc_lo
+; GFX11-NEXT:    s_delay_alu instid0(VALU_DEP_3)
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, -v1, v1, vcc_lo
 ; GFX11-NEXT:    s_setpc_b64 s[30:31]
   %i = and i32 %arg, 1
   %i3 = icmp eq i32 %i, 0
   %i4 = select i1 %i3, double %arg2, double %arg1
   %i5 = icmp sgt i32 %arg, 1
   %i6 = fneg double %i4
   %i7 = select i1 %i5, double %i6, double %i4
   ret double %i7