From 29a2b20ab363bcc0b9573e358a5ad12c0eddca86 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 4 Mar 2020 10:22:09 -0500 Subject: [PATCH] [SDAG] simplify FP binops to undef As discussed in the commit thread for rGa253a2a and D73978, we can do more undef folding for FP ops. The nnan and ninf fast-math-flags specify that if an operand is the disallowed value, the result is poison, so we can produce an undef result. But this doesn't work as expected (the undef operand cases remain) because of a Flags propagation problem in SelectionDAGBuilder. I've added DAGCombiner calls to enable these for the other cases because we've shown in other patches that (because of the limited way that SDAG iterates), it is possible to miss simplifications like this if they are done only at node creation time. Several potential follow-ups to expand on this patch are possible. Differential Revision: https://reviews.llvm.org/D75576 --- llvm/include/llvm/CodeGen/SelectionDAG.h | 3 ++- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 16 +++++++++++++ llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 21 ++++++++++++++--- llvm/test/CodeGen/AArch64/fp-const-fold.ll | 32 ++++++++++++++++---------- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h index 5e72c55..7c1dd5d 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAG.h +++ b/llvm/include/llvm/CodeGen/SelectionDAG.h @@ -1077,7 +1077,8 @@ public: /// Try to simplify a floating-point binary operation into 1 of its operands /// or a constant. - SDValue simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y); + SDValue simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y, + SDNodeFlags Flags); /// VAArg produces a result and token chain, and takes a pointer /// and a source value as input. diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 0265a6f..bca7b36 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -12220,6 +12220,9 @@ SDValue DAGCombiner::visitFADD(SDNode *N) { const TargetOptions &Options = DAG.getTarget().Options; const SDNodeFlags Flags = N->getFlags(); + if (SDValue R = DAG.simplifyFPBinop(N->getOpcode(), N0, N1, Flags)) + return R; + // fold vector ops if (VT.isVector()) if (SDValue FoldedVOp = SimplifyVBinOp(N)) @@ -12401,6 +12404,9 @@ SDValue DAGCombiner::visitFSUB(SDNode *N) { const TargetOptions &Options = DAG.getTarget().Options; const SDNodeFlags Flags = N->getFlags(); + if (SDValue R = DAG.simplifyFPBinop(N->getOpcode(), N0, N1, Flags)) + return R; + // fold vector ops if (VT.isVector()) if (SDValue FoldedVOp = SimplifyVBinOp(N)) @@ -12499,6 +12505,9 @@ SDValue DAGCombiner::visitFMUL(SDNode *N) { const TargetOptions &Options = DAG.getTarget().Options; const SDNodeFlags Flags = N->getFlags(); + if (SDValue R = DAG.simplifyFPBinop(N->getOpcode(), N0, N1, Flags)) + return R; + // fold vector ops if (VT.isVector()) { // This just handles C1 * C2 for vectors. Other vector folds are below. @@ -12831,6 +12840,9 @@ SDValue DAGCombiner::visitFDIV(SDNode *N) { const TargetOptions &Options = DAG.getTarget().Options; SDNodeFlags Flags = N->getFlags(); + if (SDValue R = DAG.simplifyFPBinop(N->getOpcode(), N0, N1, Flags)) + return R; + // fold vector ops if (VT.isVector()) if (SDValue FoldedVOp = SimplifyVBinOp(N)) @@ -12931,6 +12943,10 @@ SDValue DAGCombiner::visitFREM(SDNode *N) { ConstantFPSDNode *N0CFP = dyn_cast(N0); ConstantFPSDNode *N1CFP = dyn_cast(N1); EVT VT = N->getValueType(0); + SDNodeFlags Flags = N->getFlags(); + + if (SDValue R = DAG.simplifyFPBinop(N->getOpcode(), N0, N1, Flags)) + return R; // fold (frem c1, c2) -> fmod(c1,c2) if (N0CFP && N1CFP) diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 68bd572..53357a4 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -5229,7 +5229,7 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT, assert(VT.isFloatingPoint() && "This operator only applies to FP types!"); assert(N1.getValueType() == N2.getValueType() && N1.getValueType() == VT && "Binary operator types must match!"); - if (SDValue V = simplifyFPBinop(Opcode, N1, N2)) + if (SDValue V = simplifyFPBinop(Opcode, N1, N2, Flags)) return V; break; case ISD::FCOPYSIGN: // N1 and result must match. N1/N2 need not match. @@ -7301,9 +7301,24 @@ SDValue SelectionDAG::simplifyShift(SDValue X, SDValue Y) { return SDValue(); } -// TODO: Use fast-math-flags to enable more simplifications. -SDValue SelectionDAG::simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y) { +SDValue SelectionDAG::simplifyFPBinop(unsigned Opcode, SDValue X, SDValue Y, + SDNodeFlags Flags) { + // If this operation has 'nnan' or 'ninf' and at least 1 disallowed operand + // (an undef operand can be chosen to be Nan/Inf), then the result of this + // operation is poison. That result can be relaxed to undef. + ConstantFPSDNode *XC = isConstOrConstSplatFP(X, /* AllowUndefs */ true); ConstantFPSDNode *YC = isConstOrConstSplatFP(Y, /* AllowUndefs */ true); + bool HasNan = (XC && XC->getValueAPF().isNaN()) || + (YC && YC->getValueAPF().isNaN()); + bool HasInf = (XC && XC->getValueAPF().isInfinity()) || + (YC && YC->getValueAPF().isInfinity()); + + if (Flags.hasNoNaNs() && (HasNan || X.isUndef() || Y.isUndef())) + return getUNDEF(X.getValueType()); + + if (Flags.hasNoInfs() && (HasInf || X.isUndef() || Y.isUndef())) + return getUNDEF(X.getValueType()); + if (!YC) return SDValue(); diff --git a/llvm/test/CodeGen/AArch64/fp-const-fold.ll b/llvm/test/CodeGen/AArch64/fp-const-fold.ll index 6f0a031..b282c87 100644 --- a/llvm/test/CodeGen/AArch64/fp-const-fold.ll +++ b/llvm/test/CodeGen/AArch64/fp-const-fold.ll @@ -76,9 +76,6 @@ define double @constant_fold_fma_nan(double* %p) { define double @fdiv_nnan_nan_op0(double %x) { ; CHECK-LABEL: fdiv_nnan_nan_op0: ; CHECK: // %bb.0: -; CHECK-NEXT: mov x8, #-2251799813685248 -; CHECK-NEXT: fmov d1, x8 -; CHECK-NEXT: fdiv d0, d1, d0 ; CHECK-NEXT: ret %r = fdiv nnan double 0xfff8000000000000, %x ret double %r @@ -87,14 +84,14 @@ define double @fdiv_nnan_nan_op0(double %x) { define double @fmul_nnan_nan_op1(double %x) { ; CHECK-LABEL: fmul_nnan_nan_op1: ; CHECK: // %bb.0: -; CHECK-NEXT: mov x8, #9221120237041090560 -; CHECK-NEXT: fmov d1, x8 -; CHECK-NEXT: fmul d0, d0, d1 ; CHECK-NEXT: ret %r = fmul nnan double %x, 0x7ff8000000000000 ret double %r } +; Negative test - nan is ok. +; TODO: Should simplify to nan. + define double @fdiv_ninf_nan_op0(double %x) { ; CHECK-LABEL: fdiv_ninf_nan_op0: ; CHECK: // %bb.0: @@ -106,6 +103,9 @@ define double @fdiv_ninf_nan_op0(double %x) { ret double %r } +; Negative test - nan is ok. +; TODO: Should simplify to nan. + define double @fadd_ninf_nan_op1(double %x) { ; CHECK-LABEL: fadd_ninf_nan_op1: ; CHECK: // %bb.0: @@ -120,9 +120,6 @@ define double @fadd_ninf_nan_op1(double %x) { define double @fdiv_ninf_inf_op0(double %x) { ; CHECK-LABEL: fdiv_ninf_inf_op0: ; CHECK: // %bb.0: -; CHECK-NEXT: mov x8, #9218868437227405312 -; CHECK-NEXT: fmov d1, x8 -; CHECK-NEXT: fdiv d0, d1, d0 ; CHECK-NEXT: ret %r = fdiv ninf double 0x7ff0000000000000, %x ret double %r @@ -131,14 +128,14 @@ define double @fdiv_ninf_inf_op0(double %x) { define double @fadd_ninf_inf_op1(double %x) { ; CHECK-LABEL: fadd_ninf_inf_op1: ; CHECK: // %bb.0: -; CHECK-NEXT: mov x8, #-4503599627370496 -; CHECK-NEXT: fmov d1, x8 -; CHECK-NEXT: fadd d0, d0, d1 ; CHECK-NEXT: ret %r = fadd ninf double %x, 0xfff0000000000000 ret double %r } +; Negative test - inf is ok. +; TODO: Should simplify to inf. + define double @fsub_nnan_inf_op0(double %x) { ; CHECK-LABEL: fsub_nnan_inf_op0: ; CHECK: // %bb.0: @@ -150,6 +147,9 @@ define double @fsub_nnan_inf_op0(double %x) { ret double %r } +; Negative test - inf is ok. +; TODO: Should simplify to -inf. + define double @fmul_nnan_inf_op1(double %x) { ; CHECK-LABEL: fmul_nnan_inf_op1: ; CHECK: // %bb.0: @@ -161,6 +161,8 @@ define double @fmul_nnan_inf_op1(double %x) { ret double %r } +; TODO: Should simplify to undef + define double @fdiv_nnan_undef_op0(double %x) { ; CHECK-LABEL: fdiv_nnan_undef_op0: ; CHECK: // %bb.0: @@ -171,6 +173,8 @@ define double @fdiv_nnan_undef_op0(double %x) { ret double %r } +; TODO: Should simplify to undef + define double @fdiv_nnan_undef_op1(double %x) { ; CHECK-LABEL: fdiv_nnan_undef_op1: ; CHECK: // %bb.0: @@ -181,6 +185,8 @@ define double @fdiv_nnan_undef_op1(double %x) { ret double %r } +; TODO: Should simplify to undef + define double @fdiv_ninf_undef_op0(double %x) { ; CHECK-LABEL: fdiv_ninf_undef_op0: ; CHECK: // %bb.0: @@ -191,6 +197,8 @@ define double @fdiv_ninf_undef_op0(double %x) { ret double %r } +; TODO: Should simplify to undef + define double @fdiv_ninf_undef_op1(double %x) { ; CHECK-LABEL: fdiv_ninf_undef_op1: ; CHECK: // %bb.0: -- 2.7.4