[llvm] r237046 - propagate IR-level fast-math-flags to DAG nodes; 2nd try; NFC

Nick Lewycky nlewycky at google.com
Wed May 13 16:16:07 PDT 2015


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/20150513/c0b4209c/attachment.html>


More information about the llvm-commits mailing list