[llvm] r237046 - propagate IR-level fast-math-flags to DAG nodes; 2nd try; NFC
Sanjay Patel
spatel at rotateright.com
Thu May 14 09:18:02 PDT 2015
Thanks, Nick. I'll hope the 3rd time's the charm!
The failure case can be reduced to just this:
define double @foo(double %x, double %y) {
%div1 = fdiv fast double 1.000000e+00, %y ; remove the 'fast' and
there's no problem
%div2 = fdiv double %x, %y
%ret = fadd double %div2, %div1
ret double %ret
}
$ ./llc -enable-unsafe-fp-math crash.ll -o -
On Wed, May 13, 2015 at 5:16 PM, Nick Lewycky <nlewycky at google.com> wrote:
> On 11 May 2015 at 14:07, Sanjay Patel <spatel at rotateright.com> wrote:
>
>> Author: spatel
>> Date: Mon May 11 16:07:09 2015
>> New Revision: 237046
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=237046&view=rev
>> Log:
>> propagate IR-level fast-math-flags to DAG nodes; 2nd try; NFC
>>
>> This is a less ambitious version of:
>> http://reviews.llvm.org/rL236546
>>
>> because that was reverted in:
>> http://reviews.llvm.org/rL236600
>>
>> because it caused memory corruption that wasn't related to FMF
>> but was actually due to making nodes with 2 operands derive from a
>> plain SDNode rather than a BinarySDNode.
>>
>> This patch adds the minimum plumbing necessary to use IR-level
>> fast-math-flags (FMF) in the backend without actually using
>> them for anything yet. This is a follow-on to:
>> http://reviews.llvm.org/rL235997
>>
>> ...which split the existing nsw / nuw / exact flags and FMF
>> into their own struct.
>>
>
> I have bad news. This patch caused a regression where clang crashes.
>
> $ cat a.c
> void print(double);
> void test(double v1, double v2) {
> print(1.0 / v2);
> print(v1 / v2);
> }
> $ clang -ffast-math -O1 a.c -c -o a.o
> fatal error: error in backend: Cannot select: 0x29399e0: f64 =
> ConstantFP<1.000000e+00> [ID=14]
> In function: test
>
> on an x86-64 linux machine.
>
> I'm going to go ahead and revert this patch, but please feel free to
> reapply once it's fixed. Let me know if you need anything more from me to
> reproduce the problem!
>
> Nick
>
>
> Modified:
>> llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>> llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
>> llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>> llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Mon May 11 16:07:09
>> 2015
>> @@ -665,7 +665,7 @@ public:
>> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT);
>> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N);
>> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, SDValue
>> N2,
>> - bool nuw = false, bool nsw = false, bool exact =
>> false);
>> + const SDNodeFlags *Flags = nullptr);
>> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, SDValue
>> N2,
>> SDValue N3);
>> SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, SDValue
>> N2,
>> @@ -982,8 +982,7 @@ public:
>>
>> /// Get the specified node if it's already available, or else return
>> NULL.
>> SDNode *getNodeIfExists(unsigned Opcode, SDVTList VTs,
>> ArrayRef<SDValue> Ops,
>> - bool nuw = false, bool nsw = false,
>> - bool exact = false);
>> + const SDNodeFlags *Flags = nullptr);
>>
>> /// Creates a SDDbgValue node.
>> SDDbgValue *getDbgValue(MDNode *Var, MDNode *Expr, SDNode *N, unsigned
>> R,
>> @@ -1241,8 +1240,8 @@ private:
>> void allnodes_clear();
>>
>> BinarySDNode *GetBinarySDNode(unsigned Opcode, SDLoc DL, SDVTList VTs,
>> - SDValue N1, SDValue N2, bool nuw, bool
>> nsw,
>> - bool exact);
>> + SDValue N1, SDValue N2,
>> + const SDNodeFlags *Flags = nullptr);
>>
>> /// List of non-single value types.
>> FoldingSet<SDVTListNode> VTListMap;
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h Mon May 11
>> 16:07:09 2015
>> @@ -1017,6 +1017,11 @@ static bool isBinOpWithFlags(unsigned Op
>> case ISD::ADD:
>> case ISD::SUB:
>> case ISD::SHL:
>> + case ISD::FADD:
>> + case ISD::FDIV:
>> + case ISD::FMUL:
>> + case ISD::FREM:
>> + case ISD::FSUB:
>> return true;
>> default:
>> return false;
>> @@ -1029,8 +1034,8 @@ class BinaryWithFlagsSDNode : public Bin
>> public:
>> SDNodeFlags Flags;
>> BinaryWithFlagsSDNode(unsigned Opc, unsigned Order, DebugLoc dl,
>> SDVTList VTs,
>> - SDValue X, SDValue Y)
>> - : BinarySDNode(Opc, Order, dl, VTs, X, Y), Flags() {}
>> + SDValue X, SDValue Y, const SDNodeFlags
>> &NodeFlags)
>> + : BinarySDNode(Opc, Order, dl, VTs, X, Y), Flags(NodeFlags) {}
>> static bool classof(const SDNode *N) {
>> return isBinOpWithFlags(N->getOpcode());
>> }
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon May 11
>> 16:07:09 2015
>> @@ -1452,12 +1452,9 @@ SDValue DAGCombiner::combine(SDNode *N)
>> if (isa<ConstantSDNode>(N0) || !isa<ConstantSDNode>(N1)) {
>> SDValue Ops[] = {N1, N0};
>> SDNode *CSENode;
>> - if (const BinaryWithFlagsSDNode *BinNode =
>> - dyn_cast<BinaryWithFlagsSDNode>(N)) {
>> + if (const auto *BinNode = dyn_cast<BinaryWithFlagsSDNode>(N)) {
>> CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(),
>> Ops,
>> - BinNode->Flags.hasNoUnsignedWrap(),
>> - BinNode->Flags.hasNoSignedWrap(),
>> - BinNode->Flags.hasExact());
>> + &BinNode->Flags);
>> } else {
>> CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(),
>> Ops);
>> }
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Mon May 11
>> 16:07:09 2015
>> @@ -399,19 +399,22 @@ static void AddNodeIDOperands(FoldingSet
>> ID.AddInteger(Op.getResNo());
>> }
>> }
>> -
>> -static void AddBinaryNodeIDCustom(FoldingSetNodeID &ID, bool nuw, bool
>> nsw,
>> - bool exact) {
>> - ID.AddBoolean(nuw);
>> - ID.AddBoolean(nsw);
>> - ID.AddBoolean(exact);
>> +/// Add logical or fast math flag values to FoldingSetNodeID value.
>> +static void AddNodeIDFlags(FoldingSetNodeID &ID, unsigned Opcode,
>> + const SDNodeFlags *Flags) {
>> + if (!Flags || !isBinOpWithFlags(Opcode))
>> + return;
>> +
>> + unsigned RawFlags = Flags->getRawFlags();
>> + // If no flags are set, do not alter the ID. This saves time and allows
>> + // a gradual increase in API usage of the optional optimization flags.
>> + if (RawFlags != 0)
>> + ID.AddInteger(RawFlags);
>> }
>>
>> -/// AddBinaryNodeIDCustom - Add BinarySDNodes special infos
>> -static void AddBinaryNodeIDCustom(FoldingSetNodeID &ID, unsigned Opcode,
>> - bool nuw, bool nsw, bool exact) {
>> - if (isBinOpWithFlags(Opcode))
>> - AddBinaryNodeIDCustom(ID, nuw, nsw, exact);
>> +static void AddNodeIDFlags(FoldingSetNodeID &ID, const SDNode *N) {
>> + if (auto *Node = dyn_cast<BinaryWithFlagsSDNode>(N))
>> + AddNodeIDFlags(ID, Node->getOpcode(), &Node->Flags);
>> }
>>
>> static void AddNodeIDNode(FoldingSetNodeID &ID, unsigned short OpC,
>> @@ -506,20 +509,6 @@ static void AddNodeIDCustom(FoldingSetNo
>> ID.AddInteger(ST->getPointerInfo().getAddrSpace());
>> break;
>> }
>> - case ISD::SDIV:
>> - case ISD::UDIV:
>> - case ISD::SRA:
>> - case ISD::SRL:
>> - case ISD::MUL:
>> - case ISD::ADD:
>> - case ISD::SUB:
>> - case ISD::SHL: {
>> - const BinaryWithFlagsSDNode *BinNode =
>> cast<BinaryWithFlagsSDNode>(N);
>> - AddBinaryNodeIDCustom(
>> - ID, N->getOpcode(), BinNode->Flags.hasNoUnsignedWrap(),
>> - BinNode->Flags.hasNoSignedWrap(), BinNode->Flags.hasExact());
>> - break;
>> - }
>> case ISD::ATOMIC_CMP_SWAP:
>> case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
>> case ISD::ATOMIC_SWAP:
>> @@ -563,6 +552,8 @@ static void AddNodeIDCustom(FoldingSetNo
>> }
>> } // end switch (N->getOpcode())
>>
>> + AddNodeIDFlags(ID, N);
>> +
>> // Target specific memory nodes could also have address spaces to
>> check.
>> if (N->isTargetMemoryOpcode())
>> ID.AddInteger(cast<MemSDNode>(N)->getPointerInfo().getAddrSpace());
>> @@ -959,14 +950,16 @@ void SelectionDAG::allnodes_clear() {
>>
>> BinarySDNode *SelectionDAG::GetBinarySDNode(unsigned Opcode, SDLoc DL,
>> SDVTList VTs, SDValue N1,
>> - SDValue N2, bool nuw, bool
>> nsw,
>> - bool exact) {
>> + SDValue N2,
>> + const SDNodeFlags *Flags) {
>> if (isBinOpWithFlags(Opcode)) {
>> + // If no flags were passed in, use a default flags object.
>> + SDNodeFlags F;
>> + if (Flags == nullptr)
>> + Flags = &F;
>> +
>> BinaryWithFlagsSDNode *FN = new (NodeAllocator)
>> BinaryWithFlagsSDNode(
>> - Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs, N1, N2);
>> - FN->Flags.setNoUnsignedWrap(nuw);
>> - FN->Flags.setNoSignedWrap(nsw);
>> - FN->Flags.setExact(exact);
>> + Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs, N1, N2, *Flags);
>>
>> return FN;
>> }
>> @@ -3201,7 +3194,7 @@ SDValue SelectionDAG::FoldConstantArithm
>> }
>>
>> SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue
>> N1,
>> - SDValue N2, bool nuw, bool nsw, bool
>> exact) {
>> + SDValue N2, const SDNodeFlags *Flags) {
>> ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
>> ConstantSDNode *N2C = dyn_cast<ConstantSDNode>(N2.getNode());
>> switch (Opcode) {
>> @@ -3665,22 +3658,20 @@ SDValue SelectionDAG::getNode(unsigned O
>> // Memoize this node if possible.
>> BinarySDNode *N;
>> SDVTList VTs = getVTList(VT);
>> - const bool BinOpHasFlags = isBinOpWithFlags(Opcode);
>> if (VT != MVT::Glue) {
>> SDValue Ops[] = {N1, N2};
>> FoldingSetNodeID ID;
>> AddNodeIDNode(ID, Opcode, VTs, Ops);
>> - if (BinOpHasFlags)
>> - AddBinaryNodeIDCustom(ID, Opcode, nuw, nsw, exact);
>> + AddNodeIDFlags(ID, Opcode, Flags);
>> void *IP = nullptr;
>> if (SDNode *E = CSEMap.FindNodeOrInsertPos(ID, IP))
>> return SDValue(E, 0);
>>
>> - N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, nuw, nsw, exact);
>> + N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, Flags);
>>
>> CSEMap.InsertNode(N, IP);
>> } else {
>> - N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, nuw, nsw, exact);
>> + N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, Flags);
>> }
>>
>> InsertNode(N);
>> @@ -5984,13 +5975,12 @@ SelectionDAG::getTargetInsertSubreg(int
>> /// getNodeIfExists - Get the specified node if it's already available,
>> or
>> /// else return NULL.
>> SDNode *SelectionDAG::getNodeIfExists(unsigned Opcode, SDVTList VTList,
>> - ArrayRef<SDValue> Ops, bool nuw,
>> bool nsw,
>> - bool exact) {
>> + ArrayRef<SDValue> Ops,
>> + const SDNodeFlags *Flags) {
>> if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
>> FoldingSetNodeID ID;
>> AddNodeIDNode(ID, Opcode, VTList, Ops);
>> - if (isBinOpWithFlags(Opcode))
>> - AddBinaryNodeIDCustom(ID, nuw, nsw, exact);
>> + AddNodeIDFlags(ID, Opcode, Flags);
>> void *IP = nullptr;
>> if (SDNode *E = CSEMap.FindNodeOrInsertPos(ID, IP))
>> return E;
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Mon May
>> 11 16:07:09 2015
>> @@ -2139,6 +2139,8 @@ void SelectionDAGBuilder::visitBinary(co
>> bool nuw = false;
>> bool nsw = false;
>> bool exact = false;
>> + FastMathFlags FMF;
>> +
>> if (const OverflowingBinaryOperator *OFBinOp =
>> dyn_cast<const OverflowingBinaryOperator>(&I)) {
>> nuw = OFBinOp->hasNoUnsignedWrap();
>> @@ -2147,9 +2149,20 @@ void SelectionDAGBuilder::visitBinary(co
>> if (const PossiblyExactOperator *ExactOp =
>> dyn_cast<const PossiblyExactOperator>(&I))
>> exact = ExactOp->isExact();
>> -
>> + if (const FPMathOperator *FPOp = dyn_cast<const FPMathOperator>(&I))
>> + FMF = FPOp->getFastMathFlags();
>> +
>> + SDNodeFlags Flags;
>> + Flags.setExact(exact);
>> + Flags.setNoSignedWrap(nsw);
>> + Flags.setNoUnsignedWrap(nuw);
>> + Flags.setAllowReciprocal(FMF.allowReciprocal());
>> + Flags.setNoInfs(FMF.noInfs());
>> + Flags.setNoNaNs(FMF.noNaNs());
>> + Flags.setNoSignedZeros(FMF.noSignedZeros());
>> + Flags.setUnsafeAlgebra(FMF.unsafeAlgebra());
>> SDValue BinNodeValue = DAG.getNode(OpCode, getCurSDLoc(),
>> Op1.getValueType(),
>> - Op1, Op2, nuw, nsw, exact);
>> + Op1, Op2, &Flags);
>> setValue(&I, BinNodeValue);
>> }
>>
>> @@ -2197,9 +2210,12 @@ void SelectionDAGBuilder::visitShift(con
>> dyn_cast<const PossiblyExactOperator>(&I))
>> exact = ExactOp->isExact();
>> }
>> -
>> + SDNodeFlags Flags;
>> + Flags.setExact(exact);
>> + Flags.setNoSignedWrap(nsw);
>> + Flags.setNoUnsignedWrap(nuw);
>> SDValue Res = DAG.getNode(Opcode, getCurSDLoc(), Op1.getValueType(),
>> Op1, Op2,
>> - nuw, nsw, exact);
>> + &Flags);
>> setValue(&I, Res);
>> }
>>
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=237046&r1=237045&r2=237046&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Mon May 11
>> 16:07:09 2015
>> @@ -2660,8 +2660,9 @@ SDValue TargetLowering::BuildExactSDIV(S
>> // TODO: For UDIV use SRL instead of SRA.
>> SDValue Amt =
>> DAG.getConstant(ShAmt, dl, getShiftAmountTy(Op1.getValueType()));
>> - Op1 = DAG.getNode(ISD::SRA, dl, Op1.getValueType(), Op1, Amt, false,
>> false,
>> - true);
>> + SDNodeFlags Flags;
>> + Flags.setExact(true);
>> + Op1 = DAG.getNode(ISD::SRA, dl, Op1.getValueType(), Op1, Amt,
>> &Flags);
>> d = d.ashr(ShAmt);
>> }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150514/f4440c1c/attachment.html>
More information about the llvm-commits
mailing list