[llvm] r237046 - propagate IR-level fast-math-flags to DAG nodes; 2nd try; NFC
Nick Lewycky
nlewycky at google.com
Wed May 20 13:36:50 PDT 2015
On 20 May 2015 at 13:24, Sanjay Patel <spatel at rotateright.com> wrote:
> I think I see 2 bugs in the existing visitFDIV() code that lead to the
> crash. I don't think they are visible without the fast-math-flags patch
> though.
>
> I'll send a bug fix patch for review shortly...but this could be bad news
> for FMF in the backend. There are likely more latent bugs like this, but we
> won't hit them without applying the FMF patch first.
>
I see. Could you add FMF under a flag and then let me test it before
turning it on? I can test for compiler crashes in 24 hours, or for
miscompiles over a weekend.
Nick
On Thu, May 14, 2015 at 10:18 AM, Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>> 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/20150520/dc4fbb50/attachment.html>
More information about the llvm-commits
mailing list