DAGCombiner: Allow negating ConstantFP after legalize

[arsenm]

ping

[arsenm]

ping

[hfinkel]

Comment needs to be reworded (I provided a suggestion). Otherwise, LGTM.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

614

// Don't invert constant FP values after legalization unless the target says the negated constant is legal.

[arsenm]

r293019

include/llvm/ADT/APFloat.h

inline APFloat frexp(const APFloat &X, int &Exp, APFloat::roundingMode RM) {
   return APFloat(frexp(X.getIEEE(), Exp, RM), X.getSemantics());
 }
 /// \brief Returns the absolute value of the argument.
 inline APFloat abs(APFloat X) {
   X.clearSign();
   return X;
 }
 
+/// \brief Returns the negated value of the argument.
+inline APFloat neg(APFloat X) {
+  X.changeSign();
+  return X;
+}
+
 /// Implements IEEE minNum semantics. Returns the smaller of the 2 arguments if
 /// both are not NaN. If either argument is a NaN, returns the other argument.
 LLVM_READONLY
 inline APFloat minnum(const APFloat &A, const APFloat &B) {
   if (A.isNaN())
     return B;
   if (B.isNaN())
     return A;

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static char isNegatibleForFree(SDValue Op, bool LegalOperations,
   // Don't allow anything with multiple uses.
   if (!Op.hasOneUse()) return 0;
 
   // Don't recurse exponentially.
   if (Depth > 6) return 0;
 
   switch (Op.getOpcode()) {
   default: return false;
-  case ISD::ConstantFP:
-    // Don't invert constant FP values after legalize.  The negated constant
-    // isn't necessarily legal.
-    return LegalOperations ? 0 : 1;
+  case ISD::ConstantFP: {
+    if (!LegalOperations)
+      return 1;
+
+    // Don't invert constant FP values after legalize, unless the target the
+    // negated constant is necessarily legal.
+    EVT VT = Op.getValueType();
+    return TLI.isOperationLegal(ISD::ConstantFP, VT) ||
+      TLI.isFPImmLegal(neg(cast<ConstantFPSDNode>(Op)->getValueAPF()), VT);
+  }
   case ISD::FADD:
     // FIXME: determine better conditions for this xform.
     if (!Options->UnsafeFPMath) return 0;
 
     // After operation legalization, it might not be legal to create new FSUBs.
     if (LegalOperations &&
         !TLI.isOperationLegalOrCustom(ISD::FSUB,  Op.getValueType()))
       return 0;

test/CodeGen/AMDGPU/fneg-combines.ll

 }
 
 ; --------------------------------------------------------------------------------
 ; sin tests
 ; --------------------------------------------------------------------------------
 
 ; GCN-LABEL: {{^}}v_fneg_sin_f32:
 ; GCN: {{buffer|flat}}_load_dword [[A:v[0-9]+]]
-; GCN: v_mov_b32_e32 [[K:v[0-9]+]], 0x3e22f983
-; GCN: v_mul_f32_e64 [[MUL:v[0-9]+]], [[K]], -[[A]]
+; GCN: v_mul_f32_e32 [[MUL:v[0-9]+]], 0xbe22f983, [[A]]
 ; GCN: v_fract_f32_e32 [[FRACT:v[0-9]+]], [[MUL]]
 ; GCN: v_sin_f32_e32 [[RESULT:v[0-9]+]], [[FRACT]]
 ; GCN: buffer_store_dword [[RESULT]]
 define void @v_fneg_sin_f32(float addrspace(1)* %out, float addrspace(1)* %a.ptr) #0 {
   %tid = call i32 @llvm.amdgcn.workitem.id.x()
   %tid.ext = sext i32 %tid to i64
   %a.gep = getelementptr inbounds float, float addrspace(1)* %a.ptr, i64 %tid.ext
   %out.gep = getelementptr inbounds float, float addrspace(1)* %out, i64 %tid.ext

unittests/ADT/APFloatTest.cpp

TEST(APFloatTest, abs) {
   EXPECT_TRUE(PLargestValue.bitwiseIsEqual(abs(PLargestValue)));
   EXPECT_TRUE(PLargestValue.bitwiseIsEqual(abs(MLargestValue)));
   EXPECT_TRUE(PSmallestValue.bitwiseIsEqual(abs(PSmallestValue)));
   EXPECT_TRUE(PSmallestValue.bitwiseIsEqual(abs(MSmallestValue)));
   EXPECT_TRUE(PSmallestNormalized.bitwiseIsEqual(abs(PSmallestNormalized)));
   EXPECT_TRUE(PSmallestNormalized.bitwiseIsEqual(abs(MSmallestNormalized)));
 }
 
+TEST(APFloatTest, neg) {
+  APFloat One = APFloat(APFloat::IEEEsingle(), "1.0");
+  APFloat NegOne = APFloat(APFloat::IEEEsingle(), "-1.0");
+  APFloat Zero = APFloat::getZero(APFloat::IEEEsingle(), false);
+  APFloat NegZero = APFloat::getZero(APFloat::IEEEsingle(), true);
+  APFloat Inf = APFloat::getInf(APFloat::IEEEsingle(), false);
+  APFloat NegInf = APFloat::getInf(APFloat::IEEEsingle(), true);
+  APFloat QNaN = APFloat::getNaN(APFloat::IEEEsingle(), false);
+  APFloat NegQNaN = APFloat::getNaN(APFloat::IEEEsingle(), true);
+
+  EXPECT_TRUE(NegOne.bitwiseIsEqual(neg(One)));
+  EXPECT_TRUE(One.bitwiseIsEqual(neg(NegOne)));
+  EXPECT_TRUE(NegZero.bitwiseIsEqual(neg(Zero)));
+  EXPECT_TRUE(Zero.bitwiseIsEqual(neg(NegZero)));
+  EXPECT_TRUE(NegInf.bitwiseIsEqual(neg(Inf)));
+  EXPECT_TRUE(Inf.bitwiseIsEqual(neg(NegInf)));
+  EXPECT_TRUE(NegInf.bitwiseIsEqual(neg(Inf)));
+  EXPECT_TRUE(Inf.bitwiseIsEqual(neg(NegInf)));
+  EXPECT_TRUE(NegQNaN.bitwiseIsEqual(neg(QNaN)));
+  EXPECT_TRUE(QNaN.bitwiseIsEqual(neg(NegQNaN)));
+}
+
 TEST(APFloatTest, ilogb) {
   EXPECT_EQ(-1074, ilogb(APFloat::getSmallest(APFloat::IEEEdouble(), false)));
   EXPECT_EQ(-1074, ilogb(APFloat::getSmallest(APFloat::IEEEdouble(), true)));
   EXPECT_EQ(-1023, ilogb(APFloat(APFloat::IEEEdouble(), "0x1.ffffffffffffep-1024")));
   EXPECT_EQ(-1023, ilogb(APFloat(APFloat::IEEEdouble(), "0x1.ffffffffffffep-1023")));
   EXPECT_EQ(-1023, ilogb(APFloat(APFloat::IEEEdouble(), "-0x1.ffffffffffffep-1023")));
   EXPECT_EQ(-51, ilogb(APFloat(APFloat::IEEEdouble(), "0x1p-51")));
   EXPECT_EQ(-1023, ilogb(APFloat(APFloat::IEEEdouble(), "0x1.c60f120d9f87cp-1023")));