propagate IR-level fast-math-flags to DAG nodes

[spatel]

Summary

This patch is an attempt to introduce the minimum plumbing necessary to use IR-level fast-math-flags (FMF) in the backend along with one usage and test case (reciprocal estimate).

History:

1. IR-level FMF was added around 2012: http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-October/054999.html
2. Integer IR optimization flags (nsw, nuw, exact) were extended to the backend in June 2014: http://reviews.llvm.org/rL210467

Motivation:

1. We have customers who would like to see clang have more flexible behavior with respect to FP reciprocal approximations. This means that -freciprocal-math is honored as a separate optimization setting from -ffast-math (see existing gcc behavior for a starting point).

This is broken in the:
a. front-end: -freciprocal-math flag is ignored
b. the IR optimizer: UnsafeAlgebra implies AllowReciprocal
c. the backend: IR FMF flags are dropped entirely

2. Longer-term and more fundamentally: one of the goals for IR-level FMF was to allow mixing of strict and relaxed FP math code. This is impossible without backend support. The problem is exacerbated by LTO.

My initial draft of this patch left the nsw/nuw/exact changes mostly as-is, but it quickly became clear that maintaining the APIs for those and adding FMF on the side made everything worse. So I grouped all of the optimization flags into one class and went from there. The flags and node classes are copied directly from the IR FMF class and FPMathOperator. We should obviously unify the backend with the IR optimizer on those flag bits, but I think that can be a follow-on patch? The goal of this initial patch is to not break any existing functionality or change the IR code while adding optional processing of FMF to the backend.

I'm promising to continue work on this to convert much more of the existing backend code to use FMF wherever possible.

[andreadb]

Hi Sanjay,

I only had a couple of minor comments/questions. In general I think this is a nice initial patch.
If I remember correctly, the initial support for nsw/nuw/exact flags in SelectionDAG was added by Marcello Maggioni. In case, I suggest you to add him in cc to the code review (I don't know if he is still working on this..).

include/llvm/CodeGen/SelectionDAG.h

1230–1231 ↗

(On Diff #23427)

My understanding is that flags can only be present on binary operations. This is also the reason why originally nsw/nuw/exact were only added to BinarySDNode.
Is there a reason why BinarySDNode should extend from SDNode? At the moment, 'Ops' is always expected to have two SDValues.

include/llvm/CodeGen/SelectionDAGNodes.h

971–977 ↗

(On Diff #23427)

This is ok. However, what about having something like this?

setNoUnsignedWrap() {
  Flags |= NoUnsignedWrap;
}

clearNoUnsignedWrap() {
  Flags ^= NoUnsignedWrap;
}

Also it is a shame that most of this code is repeated in 'class SDNodeWithFlags'. I wonder if there is a better design that allows to delegate the 'bit manipulation part' as much as possible to 'SDNodeFlags'...

[spatel]

Adding Marcello to reviewers.

[spatel]

include/llvm/CodeGen/SelectionDAG.h

1230–1231 ↗

(On Diff #23427)

It is correct that we only have flags on binary ops today, but I was considering the case of adding FMF to intrinsics or libcalls ( https://llvm.org/bugs/show_bug.cgi?id=21290 ). I was also thinking about the FMA case and other nodes that don't exist in IR. I think we'll need to propagate the flags to more than binops eventually, so...

I agree that this patch is hacky in that it only updates the binop API, but it was the minimal change.

[spatel]

include/llvm/CodeGen/SelectionDAGNodes.h

971–977 ↗

(On Diff #23427)

I agree completely, but I didn't see an immediate solution, so I just went with the lazy approach: copy and paste!

I'm open to any suggestions for improvement on the interface, and eventually (soon) I would unify the IR and SDNode versions of the flags, so we get the benefits in both places.

[spatel]

re: the -freciprocal-math flag

"see existing gcc behavior for a starting point"

I should've taken my own advice more seriously. :)

It turns out that -freciprocal-math is only used for a reassociation optimization on divisions. Specifically, this one:
https://llvm.org/bugs/show_bug.cgi?id=16218

With gcc, all of the machine-level hackery that we've put into generating hardware estimate instructions is controlled by a flag that doesn't even exist in clang yet: -mrecip
https://llvm.org/bugs/show_bug.cgi?id=20912

So the end result is that this patch's recip change is wrong. Update to at least that part of the patch coming soon.

I'm assuming that we should not diverge from gcc's user-visible behavior without good reason, so we'll still make -freciprocal-math cause 'arcp' generation in IR ( http://reviews.llvm.org/rL234493 ), but 'arcp' has nothing to do with HW estimate codegen. For that, we need -mrecip with its multitude of options:
https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/i386-and-x86-64-Options.html#index-mrecip_003dopt-1627

[spatel]

Delayed by another case of 'how does anything work...ever?':
https://llvm.org/bugs/show_bug.cgi?id=23172

[spatel]

Patch updated:

1. Corrected DAGCombiner 'arcp' optimization to only trigger for the reassociation division case, not the estimate cases.
2. Updated PowerPC test case that uses that optimization (x86 still doesn't have it); worked around PR23172 by putting the 'arcp' versions of the tests before the unsafe-fp-math versions.

[kariddi]

Hello, I'm the "Marcello Maggioni" of the original backend flags by the way :P

I'm very interested in what is going on here and I think is about time we have "Node-level" Fast Math flags in the backend! This is a very nice start!

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

3618 ↗

(On Diff #23503)

Does it even make sense to have BinarySDNode around at all considering the main function generating these now generates standard SDNodes?

[spatel]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

3618 ↗

(On Diff #23503)

Hi Marcello -

Thanks for looking at this patch.

I got rid of "BinaryWithFlagsSDNode", but "BinarySDNode" had some uses elsewhere along with "UnarySDNode" and "TernarySDNode".

The comment for BinarySDNode says:

/// This class is used for two-operand SDNodes.  This is solely
/// to allow co-allocation of node operands with the node itself.

So it's just there for malloc efficiency? If everybody agrees that it has no value, then I can delete it, but I think that should be a follow-on patch.

[kariddi]

Over all as far as I'm concerned the approach here is pretty sound in my opinion and is definitely an improvement over everything we have at the moment.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

3618 ↗

(On Diff #23503)

Yeah, sure, I was just bringing up the the question, thats all :)

[echristo]

One high level comment and a few inline nits:

Can you split out the flags part of the patch first? I'm very certain that's not objectionable and will make the actual content of your patch much smaller.

Rest inline :)

Thanks!

-eric

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

7806 ↗

(On Diff #23503)

Nit: No braces around single statement ifs.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

406 ↗

(On Diff #23503)

Early exit?

416 ↗

(On Diff #23503)

Early exit?

[spatel]

Let me ask a dumb question since I don't know any better. :)

Is the size of an SDNode so critical that we can't do the proper software engineering thing: add the flags as a member rather than riding on the bonus bits available in SubclassData? If we do that, all of the distasteful get/set and bit shifting replication disappears.

If we make a bag o' bits for the flags as in this updated patch, they'd just be single byte. As it stands, I'm showing that the sizeof(SDNode) and sizeof(SDNodeWithFlags) are both 80 bytes (building on MacOSX 10.10). Free bits!

That said, there's something very wrong with this patch now...I'm seeing *intermittent* failures on seemingly unrelated codegen regression tests. Can anyone spot the bug?

[hfinkel]

In D8900#157359, @spatel wrote:

Let me ask a dumb question since I don't know any better. :)

Is the size of an SDNode so critical that we can't do the proper software engineering thing: add the flags as a member rather than riding on the bonus bits available in SubclassData? If we do that, all of the distasteful get/set and bit shifting replication disappears.

If we make a bag o' bits for the flags as in this updated patch, they'd just be single byte. As it stands, I'm showing that the sizeof(SDNode) and sizeof(SDNodeWithFlags) are both 80 bytes (building on MacOSX 10.10). Free bits!

The size is important, but if you don't increase the size on common architectures, then you should be good to go.

That said, there's something very wrong with this patch now...I'm seeing *intermittent* failures on seemingly unrelated codegen regression tests. Can anyone spot the bug?

Have you tried running under valgrind and/or asan/msan? Does this happen in a debug build, or just an optimized build?

[spatel]

In D8900#157396, @hfinkel wrote:

Have you tried running under valgrind and/or asan/msan? Does this happen in a debug build, or just an optimized build?

Not yet; that's the next step. I wanted to see if there were structural objections to this patch before chasing down the errors.

I'm seeing the failures with both debug and optimized builds. The tests that fail regularly, but not always are:

LLVM :: CodeGen/AArch64/aarch64-address-type-promotion.ll
LLVM :: CodeGen/AArch64/arm64-vector-ldst.ll
LLVM :: CodeGen/X86/cmovcmov.ll

[spatel]

In D8900#157594, @spatel wrote:

In D8900#157396, @hfinkel wrote:

Have you tried running under valgrind and/or asan/msan? Does this happen in a debug build, or just an optimized build?

Not yet; that's the next step. I wanted to see if there were structural objections to this patch before chasing down the errors.

Valgrind noted that the flags were being read uninitialized. I thought between bitfields, bools and shifts, I must've stepped into a pile of UB, but the actual bug was when creating the node. I was checking:

if (Flags && mayHaveOptimizationFlags(Opcode))

We can't use the nullptr Flags to decide the type of node to allocate; if it may have opt flags, then it must be an SDNodeWithFlags. We just have to fill in default flags in that case.

I had no luck with sanitizers (not enabled with the OS X system clang? And can't build LLVM trunk with that turned on?). But it pushed me to build on Linux x86-64 where I confirmed that the sizeof(SDNode) == sizeof(SDNodeWithFlags) == 80. So again, it looks like we can use a proper software class for the flags without any size overhead.

[spatel]

Patch updated:

1. Fixed bug when allocating SDNodesWithFlags.
2. Fixed inline problems noted by Eric Christopher.

I didn't split off the flags class from the rest of the patch as suggested by Eric on the previous review because the class is now just a trivial bit container. But if that's still the preferred way to go, please let me know.

[echristo]

Hi Sanjay,

It's still my preference if possible because it'll take the API nfc change out of the mix on the patch. Is it possible for you to do that?

Thanks!

-eric

[spatel]

In D8900#162327, @echristo wrote:

It's still my preference if possible because it'll take the API nfc change out of the mix on the patch. Is it possible for you to do that?

Hi Eric -

Sure - it's a little more work, but it's the right thing to do. I think we should actually break this into 3 steps:

1. Introduce the new Flags struct and have the existing BinaryWithFlagsSDNode use them only for nsw/nuw/exact (NFC).
2. Convert BinaryWithFlagsSDNode to the broader SDNodeWithFlags class and extend the potential users to the FP nodes (NFC).
3. Update DAGCombiner to use the 'arcp' flag and add the new test cases.

First patch coming up...

[echristo]

Absolutely agreed. Thanks!

-eric

[spatel]

Patch updated:

1. We hacked off the head of this patch into D9325 ( http://reviews.llvm.org/rL235997 ) - move the flags into their own space.
2. I've also removed the tail end of this patch; that's just the part that recognizes 'arcp' and optimizes based on it in DAGCombiner.

So this patch is now NFC-intended, but there are 2 structural changes:

1. The main diff is that we're preparing to expand the reach of the optimization flags to affect more than just binary SDNodes. I think this is already necessary for intrinsics in IR ( PR21290 - https://llvm.org/bugs/show_bug.cgi?id=21290 ). And now that we have the flags in the backend, we can propagate them to non-binop nodes that don't even exist in IR such as FMA, FNEG, etc.
2. The other change is that we're actually copying the FP fast-math-flags from the IR instructions to SDNodes. They're just not being used for anything yet...that's the next patch.

[echristo]

LGTM.

Thanks for splitting this out! :)

-eric

[spatel]

Thanks, Eric! Checked in at r236546.

[spatel]

For the record, r236546 was reverted by:
http://reviews.llvm.org/rL236600

The bug is independent of FMF or the other flags. It can be reproduced by allocating a plain SDNode for any instruction with 2 operands rather than a BinarySDNode. The bug manifests (visible under valgrind) after recycling the operand memory and/or morphing a node.

I didn't take any perf measurements, but I assume that we want to continue using the specialized operand classes (UnarySDNode, BinarySDNode, TernarySDNode) for their allocation optimizations. So I simplified this patch to keep the existing BinaryWithFlagsSDNode class that derives from BinarySDNode:
http://reviews.llvm.org/rL237046

llvm/trunk/include/llvm/CodeGen/SelectionDAG.h

SDValue getGLOBAL_OFFSET_TABLE(EVT VT) {
     return getNode(ISD::GLOBAL_OFFSET_TABLE, SDLoc(), VT);
   }
 
   /// Gets or creates the specified node.
   ///
   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,
                   SDValue N3, SDValue N4);
   SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, SDValue N1, SDValue N2,
                   SDValue N3, SDValue N4, SDValue N5);
   SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT, ArrayRef<SDUse> Ops);
   SDValue getNode(unsigned Opcode, SDLoc DL, EVT VT,
SDValue getTargetExtractSubreg(int SRIdx, SDLoc DL, EVT VT,
                                  SDValue Operand);
 
   /// A convenience function for creating TargetInstrInfo::INSERT_SUBREG nodes.
   SDValue getTargetInsertSubreg(int SRIdx, SDLoc DL, EVT VT,
                                 SDValue Operand, SDValue Subreg);
 
   /// 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,
                           bool IsIndirect, uint64_t Off, DebugLoc DL,
                           unsigned O);
 
   /// Constant
   SDDbgValue *getConstantDbgValue(MDNode *Var, MDNode *Expr, const Value *C,
SDNode *FindModifiedNodeSlot(SDNode *N, ArrayRef<SDValue> Ops,
                                void *&InsertPos);
   SDNode *UpdadeSDLocOnMergedSDNode(SDNode *N, SDLoc loc);
 
   void DeleteNodeNotInCSEMaps(SDNode *N);
   void DeallocateNode(SDNode *N);
 
   void allnodes_clear();
 
-  BinarySDNode *GetBinarySDNode(unsigned Opcode, SDLoc DL, SDVTList VTs,
-                                SDValue N1, SDValue N2, bool nuw, bool nsw,
-                                bool exact);
+  SDNode *GetSDNodeWithFlags(unsigned Opcode, SDLoc DL, SDVTList VTs,
+                             ArrayRef<SDValue> Ops, const SDNodeFlags *Flags);
 
   /// List of non-single value types.
   FoldingSet<SDVTListNode> VTListMap;
 
   /// Maps to auto-CSE operations.
   std::vector<CondCodeSDNode*> CondCodeNodes;
 
   std::vector<SDNode*> ValueTypeNodes;

llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h

public:
   /// Return a raw encoding of the flags.
   /// This function should only be used to add data to the NodeID value.
   unsigned getRawFlags() const {
     return (NoUnsignedWrap << 0) | (NoSignedWrap << 1) | (Exact << 2) |
            (UnsafeAlgebra << 3) | (NoNaNs << 4) | (NoInfs << 5) |
            (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.
 class UnarySDNode : public SDNode {
   SDUse Op;
 public:
   UnarySDNode(unsigned Opc, unsigned Order, DebugLoc dl, SDVTList VTs,
               SDValue X)
     : SDNode(Opc, Order, dl, VTs) {
     InitOperands(&Op, X);
   }
 };
 
 /// This class is used for two-operand SDNodes.  This is solely
 /// to allow co-allocation of node operands with the node itself.
 class BinarySDNode : public SDNode {
   SDUse Ops[2];
 public:
   BinarySDNode(unsigned Opc, unsigned Order, DebugLoc dl, SDVTList VTs,
                SDValue X, SDValue Y)
     : SDNode(Opc, Order, dl, VTs) {
     InitOperands(Ops, X, Y);
   }
 };
 
-/// 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 {
   SDUse Ops[3];
 public:
   TernarySDNode(unsigned Opc, unsigned Order, DebugLoc dl, SDVTList VTs,
                 SDValue X, SDValue Y, SDValue Z)
     : SDNode(Opc, Order, dl, VTs) {

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (!RV.getNode() && SelectionDAG::isCommutativeBinOp(N->getOpcode()) &&
       N->getNumValues() == 1) {
     SDValue N0 = N->getOperand(0);
     SDValue N1 = N->getOperand(1);
 
     // Constant operands are canonicalized to RHS.
     if (isa<ConstantSDNode>(N0) || !isa<ConstantSDNode>(N1)) {
       SDValue Ops[] = {N1, N0};
       SDNode *CSENode;
-      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());
+      if (auto *FlagsNode = dyn_cast<SDNodeWithFlags>(N)) {
+        CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(),
+                                      Ops, &FlagsNode->Flags);
       } else {
         CSENode = DAG.getNodeIfExists(N->getOpcode(), N->getVTList(), Ops);
       }
       if (CSENode)
         return SDValue(CSENode, 0);
     }
   }
 

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

 static void AddNodeIDOperands(FoldingSetNodeID &ID,
                               ArrayRef<SDUse> Ops) {
   for (auto& Op : Ops) {
     ID.AddPointer(Op.getNode());
     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 || !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);
 }
 
-/// 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<SDNodeWithFlags>(N))
+    AddNodeIDFlags(ID, Node->getOpcode(), &Node->Flags);
 }
 
 static void AddNodeIDNode(FoldingSetNodeID &ID, unsigned short OpC,
                           SDVTList VTList, ArrayRef<SDValue> OpList) {
   AddNodeIDOpcode(ID, OpC);
   AddNodeIDValueTypes(ID, VTList);
   AddNodeIDOperands(ID, OpList);
 }
static void AddNodeIDCustom(FoldingSetNodeID &ID, const SDNode *N) {
   }
   case ISD::STORE: {
     const StoreSDNode *ST = cast<StoreSDNode>(N);
     ID.AddInteger(ST->getMemoryVT().getRawBits());
     ID.AddInteger(ST->getRawSubclassData());
     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:
   case ISD::ATOMIC_LOAD_ADD:
   case ISD::ATOMIC_LOAD_SUB:
   case ISD::ATOMIC_LOAD_AND:
   case ISD::ATOMIC_LOAD_OR:
   case ISD::ATOMIC_LOAD_XOR:
case ISD::BlockAddress: {
     const BlockAddressSDNode *BA = cast<BlockAddressSDNode>(N);
     ID.AddPointer(BA->getBlockAddress());
     ID.AddInteger(BA->getOffset());
     ID.AddInteger(BA->getTargetFlags());
     break;
   }
   } // 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());
 }
 
 /// AddNodeIDNode - Generic routine for adding a nodes info to the NodeID
 /// data.
 static void AddNodeIDNode(FoldingSetNodeID &ID, const SDNode *N) {

 
 void SelectionDAG::allnodes_clear() {
   assert(&*AllNodes.begin() == &EntryNode);
   AllNodes.remove(AllNodes.begin());
   while (!AllNodes.empty())
     DeallocateNode(AllNodes.begin());
 }
 
-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 *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 *N = new (NodeAllocator)
-      BinarySDNode(Opcode, DL.getIROrder(), DL.getDebugLoc(), VTs, N1, N2);
+  SDNode *N = new (NodeAllocator) SDNode(Opcode, DL.getIROrder(),
+                                         DL.getDebugLoc(), VTs, Ops);
   return N;
 }
 
 void SelectionDAG::clear() {
   allnodes_clear();
   OperandAllocator.Reset();
   CSEMap.clear();
 
SDValue SelectionDAG::FoldConstantArithmetic(unsigned Opcode, SDLoc DL, EVT VT,
   // We may have a vector type but a scalar result. Create a splat.
   Outputs.resize(VT.getVectorNumElements(), Outputs.back());
 
   // Build a big vector out of the scalar elements we generated.
   return getNode(ISD::BUILD_VECTOR, SDLoc(), VT, Outputs);
 }
 
 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) {
   default: break;
   case ISD::TokenFactor:
     assert(VT == MVT::Other && N1.getValueType() == MVT::Other &&
            N2.getValueType() == MVT::Other && "Invalid token factor!");
     // Fold trivial token factors.
case ISD::OR:
       // the LHS.
       return N1;
     case ISD::SRA:
       return N1;
     }
   }
 
   // Memoize this node if possible.
-  BinarySDNode *N;
+  SDNode *N;
   SDVTList VTs = getVTList(VT);
-  const bool BinOpHasFlags = isBinOpWithFlags(Opcode);
+  SDValue Ops[] = { N1, N2 };
   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 = GetSDNodeWithFlags(Opcode, DL, VTs, Ops, Flags);
     
     CSEMap.InsertNode(N, IP);
   } else {
-    N = GetBinarySDNode(Opcode, DL, VTs, N1, N2, nuw, nsw, exact);
+    N = GetSDNodeWithFlags(Opcode, DL, VTs, Ops, Flags);
   }
 
   InsertNode(N);
   return SDValue(N, 0);
 }
 
 SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL, EVT VT,
                               SDValue N1, SDValue N2, SDValue N3) {
SelectionDAG::getTargetInsertSubreg(int SRIdx, SDLoc DL, EVT VT,
   SDNode *Result = getMachineNode(TargetOpcode::INSERT_SUBREG, DL,
                                   VT, Operand, Subreg, SRIdxVal);
   return SDValue(Result, 0);
 }
 
 /// 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;
   }
   return nullptr;
 }
 
 /// getDbgValue - Creates a SDDbgValue node.

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

 
 void SelectionDAGBuilder::visitBinary(const User &I, unsigned OpCode) {
   SDValue Op1 = getValue(I.getOperand(0));
   SDValue Op2 = getValue(I.getOperand(1));
 
   bool nuw = false;
   bool nsw = false;
   bool exact = false;
+  FastMathFlags FMF;
+  
   if (const OverflowingBinaryOperator *OFBinOp =
           dyn_cast<const OverflowingBinaryOperator>(&I)) {
     nuw = OFBinOp->hasNoUnsignedWrap();
     nsw = OFBinOp->hasNoSignedWrap();
   }
   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.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, nuw, nsw, exact);
+                                     Op1, Op2, &Flags);
   setValue(&I, BinNodeValue);
 }
 
 void SelectionDAGBuilder::visitShift(const User &I, unsigned Opcode) {
   SDValue Op1 = getValue(I.getOperand(0));
   SDValue Op2 = getValue(I.getOperand(1));
 
   EVT ShiftTy =
if (const OverflowingBinaryOperator *OFBinOp =
       nuw = OFBinOp->hasNoUnsignedWrap();
       nsw = OFBinOp->hasNoSignedWrap();
     }
     if (const PossiblyExactOperator *ExactOp =
             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);
 }
 
 void SelectionDAGBuilder::visitSDiv(const User &I) {
   SDValue Op1 = getValue(I.getOperand(0));
   SDValue Op2 = getValue(I.getOperand(1));
 
   // Turn exact SDivs into multiplications.

llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp

SDValue TargetLowering::BuildExactSDIV(SDValue Op1, SDValue Op2, SDLoc dl,
   assert(d != 0 && "Division by zero!");
 
   // Shift the value upfront if it is even, so the LSB is one.
   unsigned ShAmt = d.countTrailingZeros();
   if (ShAmt) {
     // 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);
   }
 
   // Calculate the multiplicative inverse, using Newton's method.
   APInt t, xn = d;
   while ((t = d*xn) != 1)
     xn *= APInt(d.getBitWidth(), 2) - t;
 

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

case X86::COND_O: case X86::COND_NO: {
     // Check if we really need to set the
     // Overflow flag. If NoSignedWrap is present
     // that is not actually needed.
     switch (Op->getOpcode()) {
     case ISD::ADD:
     case ISD::SUB:
     case ISD::MUL:
     case ISD::SHL: {
-      const BinaryWithFlagsSDNode *BinNode =
-          cast<BinaryWithFlagsSDNode>(Op.getNode());
-      if (BinNode->Flags.hasNoSignedWrap())
+      const SDNodeWithFlags *Node = cast<SDNodeWithFlags>(Op.getNode());
+      if (Node->Flags.hasNoSignedWrap())
         break;
     }
     default:
       NeedOF = true;
       break;
     }
     break;
   }