[llvm] r236600 - Revert r236546, "propagate IR-level fast-math-flags to DAG nodes (NFC)"

Hal Finkel hfinkel at anl.gov
Wed May 6 07:28:46 PDT 2015


----- Original Message -----
> From: "NAKAMURA Takumi" <geek4civic at gmail.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, May 6, 2015 9:03:13 AM
> Subject: [llvm] r236600 - Revert r236546,	"propagate IR-level fast-math-flags to DAG nodes (NFC)"
> 
> Author: chapuni
> Date: Wed May  6 09:03:12 2015
> New Revision: 236600
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=236600&view=rev
> Log:
> Revert r236546, "propagate IR-level fast-math-flags to DAG nodes
> (NFC)"
> 
> It caused undefined behavior.

Where? In what?

 -Hal

> 
> 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
>     llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> 
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Wed May  6
> 09:03:12 2015
> @@ -655,7 +655,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,
> -                  const SDNodeFlags *Flags = nullptr);
> +                  bool nuw = false, bool nsw = false, bool exact =
> false);
>    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,
> @@ -978,7 +978,8 @@ public:
>  
>    /// Get the specified node if it's already available, or else
>    return NULL.
>    SDNode *getNodeIfExists(unsigned Opcode, SDVTList VTs,
>    ArrayRef<SDValue> Ops,
> -                          const SDNodeFlags *Flags = nullptr);
> +                          bool nuw = false, bool nsw = false,
> +                          bool exact = false);
>  
>    /// Creates a SDDbgValue node.
>    SDDbgValue *getDbgValue(MDNode *Var, MDNode *Expr, SDNode *N,
>    unsigned R,
> @@ -1235,8 +1236,9 @@ private:
>  
>    void allnodes_clear();
>  
> -  SDNode *GetSDNodeWithFlags(unsigned Opcode, SDLoc DL, SDVTList
> VTs,
> -                             ArrayRef<SDValue> Ops, const
> SDNodeFlags *Flags);
> +  BinarySDNode *GetBinarySDNode(unsigned Opcode, SDLoc DL, SDVTList
> VTs,
> +                                SDValue N1, SDValue N2, bool nuw,
> bool nsw,
> +                                bool exact);
>  
>    /// 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=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h Wed May  6
> 09:03:12 2015
> @@ -981,44 +981,6 @@ public:
>             (NoSignedZeros << 6) | (AllowReciprocal << 7);
>    }
>  };
> -
> -/// Returns true if the opcode is an operation with optional
> optimization flags.
> -static bool mayHaveOptimizationFlags(unsigned Opcode) {
> -  switch (Opcode) {
> -  case ISD::SDIV:
> -  case ISD::UDIV:
> -  case ISD::SRA:
> -  case ISD::SRL:
> -  case ISD::MUL:
> -  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;
> -  }
> -}
> -/// This class is an extension of SDNode used from instructions that
> may have
> -/// associated extra flags.
> -class SDNodeWithFlags : public SDNode {
> -public:
> -  SDNodeFlags Flags;
> -  SDNodeWithFlags(unsigned Opc, unsigned Order, DebugLoc dl,
> SDVTList VTs,
> -                  ArrayRef<SDValue> Ops, const SDNodeFlags
> &NodeFlags)
> -    : SDNode(Opc, Order, dl, VTs, Ops) {
> -    Flags = NodeFlags;
> -  }
> -
> -  // This is used to implement dyn_cast, isa, and other type
> queries.
> -  static bool classof(const SDNode *N) {
> -    return mayHaveOptimizationFlags(N->getOpcode());
> -  }
> -};
>  
>  /// This class is used for single-operand SDNodes.  This is solely
>  /// to allow co-allocation of node operands with the node itself.
> @@ -1044,6 +1006,36 @@ public:
>    }
>  };
>  
> +/// Returns true if the opcode is a binary operation with flags.
> +static bool isBinOpWithFlags(unsigned Opcode) {
> +  switch (Opcode) {
> +  case ISD::SDIV:
> +  case ISD::UDIV:
> +  case ISD::SRA:
> +  case ISD::SRL:
> +  case ISD::MUL:
> +  case ISD::ADD:
> +  case ISD::SUB:
> +  case ISD::SHL:
> +    return true;
> +  default:
> +    return false;
> +  }
> +}
> +
> +/// This class is an extension of BinarySDNode
> +/// used from those opcodes that have associated extra flags.
> +class BinaryWithFlagsSDNode : public BinarySDNode {
> +public:
> +  SDNodeFlags Flags;
> +  BinaryWithFlagsSDNode(unsigned Opc, unsigned Order, DebugLoc dl,
> SDVTList VTs,
> +                        SDValue X, SDValue Y)
> +    : BinarySDNode(Opc, Order, dl, VTs, X, Y), Flags() { }
> +  static bool classof(const SDNode *N) {
> +    return isBinOpWithFlags(N->getOpcode());
> +  }
> +};
> +
>  /// This class is used for three-operand SDNodes. This is solely
>  /// to allow co-allocation of node operands with the node itself.
>  class TernarySDNode : public SDNode {
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed May  6
> 09:03:12 2015
> @@ -1452,9 +1452,12 @@ SDValue DAGCombiner::combine(SDNode *N)
>      if (isa<ConstantSDNode>(N0) || !isa<ConstantSDNode>(N1)) {
>        SDValue Ops[] = {N1, N0};
>        SDNode *CSENode;
> -      if (auto *FlagsNode = dyn_cast<SDNodeWithFlags>(N)) {
> -        CSENode = DAG.getNodeIfExists(N->getOpcode(),
> N->getVTList(),
> -                                      Ops, &FlagsNode->Flags);
> +      if (const BinaryWithFlagsSDNode *BinNode =
> +              dyn_cast<BinaryWithFlagsSDNode>(N)) {
> +        CSENode = DAG.getNodeIfExists(N->getOpcode(),
> N->getVTList(), Ops,
> +
>                                      BinNode->Flags.hasNoUnsignedWrap(),
> +
>                                      BinNode->Flags.hasNoSignedWrap(),
> +                                      BinNode->Flags.hasExact());
>        } 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=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Wed May  6
> 09:03:12 2015
> @@ -400,22 +400,18 @@ static void AddNodeIDOperands(FoldingSet
>    }
>  }
>  
> -/// Add logical or fast math flag values to FoldingSetNodeID value.
> -static void AddNodeIDFlags(FoldingSetNodeID &ID, unsigned Opcode,
> -                           const SDNodeFlags *Flags) {
> -  if (!Flags || !mayHaveOptimizationFlags(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);
> +static void AddBinaryNodeIDCustom(FoldingSetNodeID &ID, bool nuw,
> bool nsw,
> +                                  bool exact) {
> +  ID.AddBoolean(nuw);
> +  ID.AddBoolean(nsw);
> +  ID.AddBoolean(exact);
>  }
>  
> -static void AddNodeIDFlags(FoldingSetNodeID &ID, const SDNode *N) {
> -  if (auto *Node = dyn_cast<SDNodeWithFlags>(N))
> -    AddNodeIDFlags(ID, Node->getOpcode(), &Node->Flags);
> +/// 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 AddNodeIDNode(FoldingSetNodeID &ID, unsigned short OpC,
> @@ -510,6 +506,21 @@ 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:
> @@ -553,8 +564,6 @@ 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());
> @@ -949,22 +958,22 @@ void SelectionDAG::allnodes_clear() {
>      DeallocateNode(AllNodes.begin());
>  }
>  
> -SDNode *SelectionDAG::GetSDNodeWithFlags(unsigned Opcode, SDLoc DL,
> -                                         SDVTList VTs,
> ArrayRef<SDValue> Ops,
> -                                         const SDNodeFlags *Flags) {
> -  if (mayHaveOptimizationFlags(Opcode)) {
> -    // If no flags were passed in, use a default flags object.
> -    SDNodeFlags F;
> -    if (Flags == nullptr)
> -      Flags = &F;
> -
> -    SDNodeWithFlags *NodeWithFlags = new (NodeAllocator)
> SDNodeWithFlags(
> -      Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs, Ops, *Flags);
> -    return NodeWithFlags;
> +BinarySDNode *SelectionDAG::GetBinarySDNode(unsigned Opcode, SDLoc
> DL,
> +                                            SDVTList VTs, SDValue
> N1,
> +                                            SDValue N2, bool nuw,
> bool nsw,
> +                                            bool exact) {
> +  if (isBinOpWithFlags(Opcode)) {
> +    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);
> +
> +    return FN;
>    }
>  
> -  SDNode *N = new (NodeAllocator) SDNode(Opcode, DL.getIROrder(),
> -                                         DL.getDebugLoc(), VTs,
> Ops);
> +  BinarySDNode *N = new (NodeAllocator)
> +      BinarySDNode(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs,
> N1, N2);
>    return N;
>  }
>  
> @@ -3192,7 +3201,7 @@ SDValue SelectionDAG::FoldConstantArithm
>  }
>  
>  SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT,
>  SDValue N1,
> -                              SDValue N2, const SDNodeFlags *Flags)
> {
> +                              SDValue N2, bool nuw, bool nsw, bool
> exact) {
>    ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N1.getNode());
>    ConstantSDNode *N2C = dyn_cast<ConstantSDNode>(N2.getNode());
>    switch (Opcode) {
> @@ -3654,23 +3663,24 @@ SDValue SelectionDAG::getNode(unsigned O
>    }
>  
>    // Memoize this node if possible.
> -  SDNode *N;
> +  BinarySDNode *N;
>    SDVTList VTs = getVTList(VT);
> -  SDValue Ops[] = { N1, N2 };
> +  const bool BinOpHasFlags = isBinOpWithFlags(Opcode);
>    if (VT != MVT::Glue) {
>      SDValue Ops[] = {N1, N2};
>      FoldingSetNodeID ID;
>      AddNodeIDNode(ID, Opcode, VTs, Ops);
> -    AddNodeIDFlags(ID, Opcode, Flags);
> +    if (BinOpHasFlags)
> +      AddBinaryNodeIDCustom(ID, Opcode, nuw, nsw, exact);
>      void *IP = nullptr;
>      if (SDNode *E = CSEMap.FindNodeOrInsertPos(ID, IP))
>        return SDValue(E, 0);
>  
> -    N = GetSDNodeWithFlags(Opcode, DL, VTs, Ops, Flags);
> -
> +    N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, nuw, nsw, exact);
> +
>      CSEMap.InsertNode(N, IP);
>    } else {
> -    N = GetSDNodeWithFlags(Opcode, DL, VTs, Ops, Flags);
> +    N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, nuw, nsw, exact);
>    }
>  
>    InsertNode(N);
> @@ -5974,12 +5984,13 @@ 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,
> -                                      const SDNodeFlags *Flags) {
> +                                      ArrayRef<SDValue> Ops, bool
> nuw, bool nsw,
> +                                      bool exact) {
>    if (VTList.VTs[VTList.NumVTs - 1] != MVT::Glue) {
>      FoldingSetNodeID ID;
>      AddNodeIDNode(ID, Opcode, VTList, Ops);
> -    AddNodeIDFlags(ID, Opcode, Flags);
> +    if (isBinOpWithFlags(Opcode))
> +      AddBinaryNodeIDCustom(ID, nuw, nsw, exact);
>      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=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed
> May  6 09:03:12 2015
> @@ -2139,8 +2139,6 @@ 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();
> @@ -2150,20 +2148,8 @@ void SelectionDAGBuilder::visitBinary(co
>            dyn_cast<const PossiblyExactOperator>(&I))
>      exact = ExactOp->isExact();
>  
> -  if (const FPMathOperator *FPOp = dyn_cast<const
> FPMathOperator>(&I))
> -    FMF = FPOp->getFastMathFlags();
> -
> -  SDNodeFlags Flags;
> -  Flags.setAllowReciprocal(FMF.allowReciprocal());
> -  Flags.setExact(exact);
> -  Flags.setNoInfs(FMF.noInfs());
> -  Flags.setNoNaNs(FMF.noNaNs());
> -  Flags.setNoSignedWrap(nsw);
> -  Flags.setNoSignedZeros(FMF.noSignedZeros());
> -  Flags.setNoUnsignedWrap(nuw);
> -  Flags.setUnsafeAlgebra(FMF.unsafeAlgebra());
>    SDValue BinNodeValue = DAG.getNode(OpCode, getCurSDLoc(),
>    Op1.getValueType(),
> -                                     Op1, Op2, &Flags);
> +                                     Op1, Op2, nuw, nsw, exact);
>    setValue(&I, BinNodeValue);
>  }
>  
> @@ -2212,12 +2198,8 @@ void SelectionDAGBuilder::visitShift(con
>        exact = ExactOp->isExact();
>    }
>  
> -  SDNodeFlags Flags;
> -  Flags.setExact(exact);
> -  Flags.setNoSignedWrap(nsw);
> -  Flags.setNoUnsignedWrap(nuw);
>    SDValue Res = DAG.getNode(Opcode, getCurSDLoc(),
>    Op1.getValueType(), Op1, Op2,
> -                            &Flags);
> +                            nuw, nsw, exact);
>    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=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Wed May  6
> 09:03:12 2015
> @@ -2660,9 +2660,8 @@ SDValue TargetLowering::BuildExactSDIV(S
>      // TODO: For UDIV use SRL instead of SRA.
>      SDValue Amt = DAG.getConstant(ShAmt, dl,
>                                    getShiftAmountTy(Op1.getValueType()));
> -    SDNodeFlags Flags;
> -    Flags.setExact(true);
> -    Op1 = DAG.getNode(ISD::SRA, dl, Op1.getValueType(), Op1, Amt,
> &Flags);
> +    Op1 = DAG.getNode(ISD::SRA, dl, Op1.getValueType(), Op1, Amt,
> false, false,
> +                      true);
>      d = d.ashr(ShAmt);
>    }
>  
> 
> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=236600&r1=236599&r2=236600&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Wed May  6 09:03:12
> 2015
> @@ -12561,8 +12561,9 @@ SDValue X86TargetLowering::EmitTest(SDVa
>      case ISD::SUB:
>      case ISD::MUL:
>      case ISD::SHL: {
> -      const SDNodeWithFlags *Node =
> cast<SDNodeWithFlags>(Op.getNode());
> -      if (Node->Flags.hasNoSignedWrap())
> +      const BinaryWithFlagsSDNode *BinNode =
> +          cast<BinaryWithFlagsSDNode>(Op.getNode());
> +      if (BinNode->Flags.hasNoSignedWrap())
>          break;
>      }
>      default:
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list