[FPEnv] Don't transform FSUB(-0, X) -> FNEG(X) in SelectionDAGBuilder.

[cameron.mcinally]

Summary

This is a subset of the patch in D73978. Hopefully this change is easier to digest...

This patch stops unconditionally transforming FSUB(-0, X) into an FNEG(X) while building the DAG. There are also two small changes to handle the new FSUB(-0,X) similarly to FNEG(X).

I've left some NOTE comments in the tests to illustrate the assembly changes, since I'm not that familiar with AMDGPU. These are just to ease reviewing. I'll remove those comments if the asm looks acceptable.

[arsenm]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

I thought the point was this cannot be lowered this way?

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

39

This is a code size regression. We probably need

This should just change to use fneg. We could probably consider the fsub case in performFNegCombine though

[cameron.mcinally]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

This isn't as bad as it seems, I think. This is isCanonicalized(...), so we're just saying the the FSUB(-0,X) *might* not flush denormals (if DAGCombine wants to change it to an FNEG(X) later).

We could check the DenormalMode here though. That would let us know what will happen to the FSUB(-0,X) in DAGCombine. Assuming it's not a can of worms, I could do that. If it is a can-of-worms, might be better left for separate patch. It should be functional with this patch, just not optimal.

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

39

This should just change to use fneg.

Are you saying the IR in this test should just be updated to use FNEG?

I'll look to see if there's a clean way to combine FSUB with the FMED...

[cameron.mcinally]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

39

This problem is really sticky (and maybe a design flaw in DAGCombine). The initial DAGs for pre/post-Diff are largely the same, except for the FSUB/FNEG node.

The problem arrises in visitFSUB(...), when the FSUB(-0,X) is transformed to FNEG(X). This combine, of course, removes the FSUB(-0,X) from its current position in the Worklist, and the new FNEG(X) is placed at the end of the Worklist.

This new Worklist order is pretty unfortunate. The two transforms that are needed to fold the FNEG into the FMED3 are now out of order. (If DAGCombine ran one more time, it looks like we'd correctly get the fold.)

[cameron.mcinally]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

39

Actually, this is a bug in AMDGPUTargetLowering::performFNegCombine(...). Under the FMED3 case, we have this code:

if (Res.getOpcode() != AMDGPUISD::FMED3)
  return SDValue(); // Op got folded away.
if (!Op.hasOneUse()) 
  DAG.ReplaceAllUsesWith(Op, DAG.getNode(ISD::FNEG, SL, VT, Res));
return Res;

That's a problem. We're using RAUW to insert the new user(s) of Res. But we return Res, not the new user(s), to DAGCombine.

DAGCombine will then add the users of Res back to the Worklist, but not the users of the new FNEG. So we miss the new FNEG combine opportunity.

More details to come tomorrow...

[cameron.mcinally]

Fix the FMED3 DAGCombine problem by explicitly adding the new node's users to the Worklist.

Heads up, @arsenm. There are other instances of this bug in performFNegCombine(...). I see at least two in the other FMA cases.

[cameron.mcinally]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

We could check the DenormalMode here though.

Guarding this code based on denormal mode is easy enough, but it affects the assembly in a number of tests where denormals are enabled. That will make this patch harder to review. It might be better to optimize in a stand-alone Diff.

Thoughts on this?

[arsenm]

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

39

Yes, the IR should be changed. Theoretically the fsub case would be another test and a different optimization

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

3796–3797 ↗

(On Diff #279523)

I don't actually know why this would ever happen?

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

I think checking for denormals isn't quite correct; canonicalized doesn't just mean denormals are flushed. It also means at minimum signaling nans are quieted

[cameron.mcinally]

llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp

3796–3797 ↗

(On Diff #279523)

Came from D50706. The IR for this particular problem is:

%med3 = call float @llvm.amdgcn.fmed3.f32(float %src0, float %src1, float %src2)
%neg.med3 = fsub float -0.0, %med3
%med3.user = fmul float %med3, 4.0

The intention is that the FNEG is sunk into the FMED3 operands and the FMUL absorbs it by negating the constant.

Seems like a pretty specific peep to me, but I'm no expert on this instruction...

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

That's true, but there is more nuance here...

For the general FSUB case, the result will be canonicalized. No problem there.

For the FSUB(-0,X) case, the result may or may not be canonicalized, depending on if the FSUB is replaced with a FNEG or not. And DAGCombine will only do that FNEG transform if in a particular denormal mode.

So, as long as both this check and the decision to transform FSUB(-0,X)->FNEG(X) are in lockstep, it's not really a problem. But, clearly, this is somewhat brittle and error prone.

Being a little pessimistic in order to more forward isn't the worst thing, assuming we don't regress too much. Figuring out a good solution to this subproblem will be easier (for me at least) if we can isolate it from the larger project.

[cameron.mcinally]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

This is the test that's tripping:

; GCN-LABEL: {{^}}v_test_canonicalize_fneg_fabs_var_f32:
; GFX678: v_mul_f32_e64 [[REG:v[0-9]+]], -1.0, |{{v[0-9]+}}|
; GFX9: v_max_f32_e64 [[REG:v[0-9]+]], -|{{v[0-9]+}}|, -|{{v[0-9]+}}|
; GCN: {{flat|global}}_store_dword v{{\[[0-9]+:[0-9]+\]}}, [[REG]]
define amdgpu_kernel void @v_test_canonicalize_fneg_fabs_var_f32(float addrspace(1)* %out) #1 {
  %val = load float, float addrspace(1)* %out
  %val.fabs = call float @llvm.fabs.f32(float %val)
  %val.fabs.fneg = fsub float -0.0, %val.fabs
  %canonicalized = call float @llvm.canonicalize.f32(float %val.fabs.fneg)
  store float %canonicalized, float addrspace(1)* %out
  ret void
}

attributes #1 = { nounwind "denormal-fp-math-f32"="preserve-sign,preserve-sign" }

We need a way for isCanonicalized(...) to return that an FSUB(-0,X) might not canonicalize if it is combined into an FNEG. Otherwise, the @llvm.canonicalize.f32 will be removed and we end up with:

flat_load_dword v2, v[0:1]
s_waitcnt vmcnt(0) lgkmcnt(0)
v_or_b32_e32 v2, 0x80000000, v2
flat_store_dword v[0:1], v2

Maybe we need a isFSUBtoFNEGLegal(...) helper function? That way the check is uniform across all uses?

[arsenm]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

Basically none of these tests you're running into are intended to use fsub. They're all fnegs that weren't ported since that's what the source modifiers are intended to directly match. Trying to do anything smarter with folding fsub's is an optimization beyond what this intended to check. I think we should just start from the position that it is not legal to lower fsub -0, x to fneg and work from there

[cameron.mcinally]

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

9274–9275

Basically none of these tests you're running into are intended to use fsub. They're all fnegs that weren't ported since that's what the source modifiers are intended to directly match. Trying to do anything smarter with folding fsub's is an optimization beyond what this intended to check.

Ok, that's fair. To reiterate, the tests should be updated to use FNEG. I'll prepare that patch for you to check out. I apologize for putting that burden on you, but I have very little intuition about the AMDGPU backend, so I want to change as little as possible.

I think we should just start from the position that it is not legal to lower fsub -0, x to fneg and work from there

I think this is the right thing to do, assuming we don't impact out-of-tree targets that might still generate the old FSUB(-0,X) idiom.

Thinking some more, I'm fairly sure there are bugs in opt that mistakenly generate the old FSUB(-0,X) pattern. So those will need to be sorted out too. And I'm not sure we can get away with bulk updating all the llc tests for other targets to use FNEG over FSUB(-0,X).

Threading a needle....

[cameron.mcinally]

Update the tests failing from the SelectionDAGBuilder change to use FNEG.

[arsenm]

LGTM. Can you also make the same fix in GlobalISel?

[craig.topper]

I think this needs to be done in FastISel too?

[cameron.mcinally]

Apologies for the slow reply. I was on vacation last week...

Thanks. Will do. I still need to unwind the unconditional FNEG transform in DAGCombine too.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h

private:
   void visitUnary(const User &I, unsigned Opcode);
   void visitFNeg(const User &I) { visitUnary(I, ISD::FNEG); }
 
   void visitBinary(const User &I, unsigned Opcode);
   void visitShift(const User &I, unsigned Opcode);
   void visitAdd(const User &I)  { visitBinary(I, ISD::ADD); }
   void visitFAdd(const User &I) { visitBinary(I, ISD::FADD); }
   void visitSub(const User &I)  { visitBinary(I, ISD::SUB); }
-  void visitFSub(const User &I);
+  void visitFSub(const User &I) { visitBinary(I, ISD::FSUB); }
   void visitMul(const User &I)  { visitBinary(I, ISD::MUL); }
   void visitFMul(const User &I) { visitBinary(I, ISD::FMUL); }
   void visitURem(const User &I) { visitBinary(I, ISD::UREM); }
   void visitSRem(const User &I) { visitBinary(I, ISD::SREM); }
   void visitFRem(const User &I) { visitBinary(I, ISD::FREM); }
   void visitUDiv(const User &I) { visitBinary(I, ISD::UDIV); }
   void visitSDiv(const User &I);
   void visitFDiv(const User &I) { visitBinary(I, ISD::FDIV); }

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (&I != &BB.front()) {
           return;
       }
     }
   }
 
   DAG.setRoot(DAG.getNode(ISD::TRAP, getCurSDLoc(), MVT::Other, DAG.getRoot()));
 }
 
-void SelectionDAGBuilder::visitFSub(const User &I) {
-  // -0.0 - X --> fneg
-  Type *Ty = I.getType();
-  if (isa<Constant>(I.getOperand(0)) &&
-      I.getOperand(0) == ConstantFP::getZeroValueForNegation(Ty)) {
-    SDValue Op2 = getValue(I.getOperand(1));
-    setValue(&I, DAG.getNode(ISD::FNEG, getCurSDLoc(),
-                             Op2.getValueType(), Op2));
-    return;
-  }
-
-  visitBinary(I, ISD::FSUB);
-}
-
 void SelectionDAGBuilder::visitUnary(const User &I, unsigned Opcode) {
   SDNodeFlags Flags;
 
   SDValue Op = getValue(I.getOperand(0));
   SDValue UnNodeValue = DAG.getNode(Opcode, getCurSDLoc(), Op.getValueType(),
                                     Op, Flags);
   setValue(&I, UnNodeValue);
 }

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
                                              NegatibleCost &Cost,
                                              unsigned Depth) const {
   // fneg is removable even if it has multiple uses.
   if (Op.getOpcode() == ISD::FNEG) {
     Cost = NegatibleCost::Cheaper;
     return Op.getOperand(0);
   }
 
+  // Treat fsub(-0.0,x) the same as fneg.
+  if (Op.getOpcode() == ISD::FSUB) {
+    ConstantFPSDNode *N0C = isConstOrConstSplatFP(Op.getOperand(0), true);
+    if (N0C && N0C->isZero()) {
+      // TODO: Handle NSZ cases.
+      if (N0C->isNegative()) {
+        Cost = NegatibleCost::Cheaper;
+        return Op.getOperand(1);
+      }
+    }
+  }
+
   // Don't recurse exponentially.
   if (Depth > SelectionDAG::MaxRecursionDepth)
     return SDValue();
 
   // Pre-increment recursion depth for use in recursive calls.
   ++Depth;
   const SDNodeFlags Flags = Op->getFlags();
   const TargetOptions &Options = DAG.getTarget().Options;

llvm/lib/Target/AMDGPU/SIISelLowering.cpp

bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
   // If source is a result of another standard FP operation it is already in
   // canonical form.
   if (MaxDepth == 0)
     return false;
 
   switch (Opcode) {
   // These will flush denorms if required.
   case ISD::FADD:
-  case ISD::FSUB:
   case ISD::FMUL:
   case ISD::FCEIL:
   case ISD::FFLOOR:
   case ISD::FMA:
   case ISD::FMAD:
   case ISD::FSQRT:
   case ISD::FDIV:
   case ISD::FREM:
bool SITargetLowering::isCanonicalized(SelectionDAG &DAG, SDValue Op,
   case AMDGPUISD::LDEXP:
   case AMDGPUISD::CVT_PKRTZ_F16_F32:
   case AMDGPUISD::CVT_F32_UBYTE0:
   case AMDGPUISD::CVT_F32_UBYTE1:
   case AMDGPUISD::CVT_F32_UBYTE2:
   case AMDGPUISD::CVT_F32_UBYTE3:
     return true;
 
+  case ISD::FSUB: {
+    // FSUB(-0.0,X) can be lowered or combined as a bit operation.
+    // Need to check its input recursively to handle.
+    ConstantFPSDNode *N0C = isConstOrConstSplatFP(Op.getOperand(0), true);
+    // TODO: Handle NSZ.
+    if (N0C && N0C->isZero() && N0C->isNegative())
+      return isCanonicalized(DAG, Op.getOperand(1), MaxDepth - 1);
+
+    return true;
+  }
   // It can/will be lowered or combined as a bit operation.
   // Need to check their input recursively to handle.
   case ISD::FNEG:
   case ISD::FABS:
   case ISD::FCOPYSIGN:
     return isCanonicalized(DAG, Op.getOperand(0), MaxDepth - 1);
 
   case ISD::FSIN:

llvm/test/CodeGen/AMDGPU/fdiv32-to-rcp-folding.ll

 ; GCN:            global_store_dwordx4
 define amdgpu_kernel void @div_v4_c_by_x_25ulp(<4 x float> addrspace(1)* %arg) {
   %load = load <4 x float>, <4 x float> addrspace(1)* %arg, align 16
   %div = fdiv <4 x float> <float 2.000000e+00, float 1.000000e+00, float -1.000000e+00, float -2.000000e+00>, %load, !fpmath !0
   store <4 x float> %div, <4 x float> addrspace(1)* %arg, align 16
   ret void
 }
 
+; FIXME: These changes look supicious, but they seem ok. It looks like non-determinism in
+;        DAGCombine is picking a different lowering, but it's the same instructions and counts.
+;        Although, there is a small scheduling difference with a move towards the end.
 ; GCN-LABEL: {{^}}div_v4_c_by_minus_x_25ulp:
-; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, -2.0{{$}}
-; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, -2.0{{$}}
-; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, -2.0{{$}}
-; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, -2.0{{$}}
+; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, 2.0{{$}}
+; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, 2.0{{$}}
+; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, 2.0{{$}}
+; GCN-DENORM-DAG: v_div_scale_f32 {{.*}}, 2.0{{$}}
 ; GCN-DENORM-DAG: v_rcp_f32_e32
 ; GCN-DENORM-DAG: v_rcp_f32_e32
 
 ; GCN-DAG:        v_mov_b32_e32 [[L:v[0-9]+]], 0x6f800000
 ; GCN-DAG:        v_mov_b32_e32 [[S:v[0-9]+]], 0x2f800000
 
 ; GCN-DAG:        v_cmp_gt_f32_e64 vcc, |s{{[0-9]+}}|, [[L]]
 ; GCN-DAG:        v_cndmask_b32_e32 v{{[0-9]+}}, 1.0, [[S]], vcc
 ; GCN-DAG:        v_cmp_gt_f32_e64 vcc, |s{{[0-9]+}}|, [[L]]
 ; GCN-DAG:        v_cndmask_b32_e32 v{{[0-9]+}}, 1.0, [[S]], vcc
 
 ; GCN-DENORM-DAG: v_mul_f32_e32 v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
 ; GCN-DENORM-DAG: v_mul_f32_e64 v{{[0-9]+}}, -s{{[0-9]+}}, v{{[0-9]+}}
 ; GCN-DENORM-DAG: v_rcp_f32_e32 [[RCP1:v[0-9]+]], v{{[0-9]+}}
 ; GCN-DENORM-DAG: v_mul_f32_e32 v{{[0-9]+}}, v{{[0-9]+}}, [[RCP1]]
 ; GCN-DENORM-DAG: v_rcp_f32_e32 [[RCP2:v[0-9]+]], v{{[0-9]+}}
 ; GCN-DENORM-DAG: v_mul_f32_e32 v{{[0-9]+}}, v{{[0-9]+}}, [[RCP2]]
 
 ; GCN-DENORM-DAG: v_div_fmas_f32
 ; GCN-DENORM-DAG: v_div_fmas_f32
-; GCN-DENORM-DAG: v_div_fixup_f32 {{.*}}, -2.0{{$}}
-; GCN-DENORM-DAG: v_div_fixup_f32 {{.*}}, -2.0{{$}}
+; GCN-DENORM-DAG: v_div_fixup_f32 {{.*}}, 2.0{{$}}
+; GCN-DENORM-DAG: v_div_fixup_f32 {{.*}}, 2.0{{$}}
 
 ; GCN-FLUSH-DAG:  v_rcp_f32_e32
 ; GCN-FLUSH-DAG:  v_rcp_f32_e64
 
 ; GCN-NOT:        v_cmp_gt_f32_e64
 ; GCN-NOT:        v_cndmask_b32_e32
 ; GCN-FLUSH-NOT:  v_div
 

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.fmed3.ll

 ; GCN: v_med3_f32 v{{[0-9]+}}, -s{{[0-9]+}}, -v{{[0-9]+}}, -v{{[0-9]+}}
 define amdgpu_kernel void @test_fneg_fmed3(float addrspace(1)* %out, float %src0, float %src1, float %src2) #1 {
   %med3 = call float @llvm.amdgcn.fmed3.f32(float %src0, float %src1, float %src2)
   %neg.med3 = fsub float -0.0, %med3
   store float %neg.med3, float addrspace(1)* %out
   ret void
 }
 
+; NOTE: Seems equivalent, but I'm not sure. The instruction changed.
 ; GCN-LABEL: {{^}}test_fneg_fmed3_multi_use:
 ; GCN: v_med3_f32 [[MED3:v[0-9]+]], -s{{[0-9]+}}, -v{{[0-9]+}}, -v{{[0-9]+}}
-; GCN: v_mul_f32_e32 v{{[0-9]+}}, -4.0, [[MED3]]
+; GCN: v_mul_f32_e64 v{{[0-9]+}}, -[[MED3]], 4.0
 define amdgpu_kernel void @test_fneg_fmed3_multi_use(float addrspace(1)* %out, float %src0, float %src1, float %src2) #1 {
   %med3 = call float @llvm.amdgcn.fmed3.f32(float %src0, float %src1, float %src2)
   %neg.med3 = fsub float -0.0, %med3
   %med3.user = fmul float %med3, 4.0
   store volatile float %med3.user, float addrspace(1)* %out
   store volatile float %neg.med3, float addrspace(1)* %out
   ret void
 }
 
 ; GCN-LABEL: {{^}}test_fabs_fmed3:
 ; GCN: v_med3_f32 [[MED3:v[0-9]+]], s{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}

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

 ; RUN: llc -march=amdgcn < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
 ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s
 
 
 ; FUNC-LABEL: {{^}}test_a:
 ; EG-NOT: CND
 ; EG: SET{{[NEQGTL]+}}_DX10
 
+; NOTE: Don't think we can make FSUB(-0,X) work here. The FSUB(-0,X) is
+;       being combined with the SELECT. Can't prevent that xform without
+;       affecting other targets.
 define amdgpu_kernel void @test_a(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp olt float %in, 0.000000e+00
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   %4 = bitcast i32 %3 to float
   %5 = bitcast float %4 to i32
   %6 = icmp ne i32 %5, 0
   br i1 %6, label %IF, label %ENDIF
 
 IF:
   %7 = getelementptr i32, i32 addrspace(1)* %out, i32 1

 ; EG-LABEL: {{^}}test_b:
 ; EG: SET{{[GTEQN]+}}_DX10
 ; EG-NEXT: PRED_
 ; EG-NEXT: ALU clause starting
 define amdgpu_kernel void @test_b(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp olt float %in, 0.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   %4 = bitcast i32 %3 to float
   %5 = bitcast float %4 to i32
   %6 = icmp ne i32 %5, 0
   br i1 %6, label %ENDIF, label %IF
 
 IF:
   %7 = getelementptr i32, i32 addrspace(1)* %out, i32 1

llvm/test/CodeGen/AMDGPU/set-dx10.ll

 ; RUN: llc < %s -march=r600 -mcpu=redwood | FileCheck %s
 
 ; These tests check that floating point comparisons which are used by select
 ; to store integer true (-1) and false (0) values are lowered to one of the
 ; SET*DX10 instructions.
 
 ; CHECK: {{^}}fcmp_une_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETNE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_une_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp une float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_une_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETNE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,

 ; CHECK: {{^}}fcmp_oeq_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_oeq_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp oeq float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_oeq_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,

 ; CHECK: {{^}}fcmp_ogt_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGT_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_ogt_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp ogt float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_ogt_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGT_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,

 ; CHECK: {{^}}fcmp_oge_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_oge_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp oge float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_oge_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].Z, literal.y,

 ; CHECK: {{^}}fcmp_ole_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, literal.y, KC0[2].Z,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_ole_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp ole float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_ole_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGE_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, literal.y, KC0[2].Z,

 ; CHECK: {{^}}fcmp_olt_select_fptosi:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGT_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, literal.y, KC0[2].Z,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_olt_select_fptosi(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp olt float %in, 5.0
   %1 = select i1 %0, float 1.000000e+00, float 0.000000e+00
-  %2 = fsub float -0.000000e+00, %1
+  %2 = fneg float %1
   %3 = fptosi float %2 to i32
   store i32 %3, i32 addrspace(1)* %out
   ret void
 }
 
 ; CHECK: {{^}}fcmp_olt_select_i32:
 ; CHECK: LSHR
 ; CHECK-NEXT: SETGT_DX10 * {{\** *}}T{{[0-9]+\.[XYZW]}}, literal.y, KC0[2].Z,
 ; CHECK-NEXT: 1084227584(5.000000e+00)
 define amdgpu_kernel void @fcmp_olt_select_i32(i32 addrspace(1)* %out, float %in) {
 entry:
   %0 = fcmp olt float %in, 5.0
   %1 = select i1 %0, i32 -1, i32 0
   store i32 %1, i32 addrspace(1)* %out
   ret void
 }