[SelectionDAG] Use a union of bitfield structs for SDNode::SubclassData.

[jlebar]

Summary

This greatly simplifies our handling of SDNode::SubclassData.

NFC, hopefully. :)

[jlebar]

Update per bitfield API discussion in D23035.

[chandlerc]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

408–411

How much are getters and setters at this layer really providing?

If we need these, I'd expect to want them in the actual SDNode public API. For participants in the SDNode type hierarchy, they don't seem to add a lot of value over directly referencing the bitfield.

413–414

You can't actually do this. An underscore followed by a capital letter is a reserved identifier.

But see above, I don't think we need to do this.

415

using an explicitly *sized* type for a bitfield seems... actively confusing. I'd just use 'unsigned'.

[jlebar]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

408–411

How much are getters and setters at this layer really providing?

I think it was your idea! :)

The notion iirc was that

• we want wrappers for writing enum type-fields in order to check that we're not writing a too-wide value,
• we want wrappers for reading strongly-typed enum-type fields so we don't have to static_cast everywhere,
• we wanted to be consistent with these for the bool fields, and
• we get some type-safety on the bool fields this way too.

If we need these, I'd expect to want them in the actual SDNode public API

I don't quite see how that would work. We don't want to expose all of these getters in SDNode itself because some are only valid to read if we're e.g. a MemSDNode or whatever.

[chandlerc]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

408–411

I think I was trying to suggest that accessors would make sense on the *public* API...

And in fact, the ones that we need it looks like are already there, so this is probably just already the case today.

I'm fine if we need a few to dodge static_cast goop for enums, but for all the bools I would just directly access them from within the SDNode type hierarchy code.

[jlebar]

Get rid of getters/setters entirely.

FWIW I'm not totally wild about this, because the assertions are verbose. It
brings me back to the earlier idea in D23035 of using a macro to combine the
assignment and assertion.

But I'm happy to leave this how it is too.

[jlebar]

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

415

They pack differently? https://godbolt.org/g/dwzPfq

We're trying to pack these with an int16_t (SDNode::NodeType).

[jlebar]

Friendly ping.

[chandlerc]

Mostly minor stuff. This is pretty close I think.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

415

Cool, comment at the top of the list of bitfields that:

a) We define a set of mini helper classes to hold all the bitfields used.
b) The total is designed to pack with an int16_t and so shouldn't exceed 16-bits and should uint16_t as the type.

450–456

Is this just a getter/setter pair you missed or was this specifically intended to deviate from the pattern? (If the latter, a comment would be good.)

1244–1245

Would the form using the accessor have the same effect:

assert(getOrdering() == SuccessOrdering && "Value truncated");

1247

If this compiles, then the above could probably be written this way as well.

I'm fine with whichever way of writing this accomplishes your intent and is shorter.

1818–1819

Where did these asserts go? I may just be missing the reason they're already covered, sorry if so...

1846–1847

Just marking the other place where asserts seem to be lost.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

5086–5092

I think you should preface this with an explanation of why you're building a node at all:

// We build a synthetic LoadSDNode to extract its subclass data as
// an integer for the folding set. The compiler can reduce ...

That will help the reader make sense of the subclass data being relevant when they've not yet gotten to the member function call on the temporary LoadSDNode.

Actually, I see that this pattern repeats itself a bunch. Not sure how best to handle it as the repeated comment would indeed become quite tedious.

My only idea would be to keep a helper routine but make it look more like:

template <typename SDNodeT, typename ...Ts>
uint16_t getSyntheticNodeSubclassData(const SDLoc &dl, Ts&&... Args) {
  return SDNodeT(dl.getIROrder(), DebugLoc(), std::forward<Ts>(Args)...)
             .getRawSubclassData();
}

And then you could attach your comment there. But that's a mighty magical helper.

Anyways, I don't think this is a blocking issue if you don't like this idea and can't come up with anything better.

[jlebar]

Second round of chandlerc's review.

[jlebar]

Thank you for the review, Chandler.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

450–456

Yes, just one that got away.

1818–1819

I think that was overaggressive pruning on my part, brought them back.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

5086–5092

Personally I like it as a place to hang the comment and don't mind the magic.

I had to make the helper a member function of SelectionDAG, because the nodes' constructors are all private. I wasn't wild about that, but then I saw that we already had a function there for creating SDNodes. So I think it fits ok.

[chandlerc]

Two tiny points below but LGTM with those fixed. Thanks for doing this it's a huge improvement!

llvm/include/llvm/CodeGen/SelectionDAG.h

279 ↗

(On Diff #69008)

Three slashes for doxygen

286 ↗

(On Diff #69008)

Make this static?

[ahatanak]

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

1103 ↗

(On Diff #69014)

I think this assert is not correct for some targets. sizeof(SDNodeBits) is equal to 1 if the target is "-arch armv7" because the alignment of class SDNodeBitfields is 1-byte. If you compile the following code with "-arch armv7", you'll see function foo1 returns 1.

class S1 {
  short HasDebugValue : 1;
  short IsMemIntrinsic : 1;
};

unsigned foo1() {
  S1 s1;
  return sizeof(s1);
}

[ahatanak]

Justin, I have to fix this soon since there is an internal bot that is failing the assert, which I suspect is happening because sizeof(SDNodeBits) returns 1. It doesn't look like the correctness of the code depends on sizeof(SDNodeBits) being equal to sizeof(unit16_t), so I'll just remove the assert or replace "==" with "<=".

[ahatanak]

Fixed the static_assert in r279797. Feel free to change the message or remove the assert if you want. I'll be watching the bot to see if it passes the static_assert.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

 
 /// Represents one node in the SelectionDAG.
 ///
 class SDNode : public FoldingSetNode, public ilist_node<SDNode> {
 private:
   /// The operation that this node performs.
   int16_t NodeType;
 
-  /// This tracks whether this node has one or more dbg_value
-  /// nodes corresponding to it.
-  uint16_t HasDebugValue : 1;
-
 protected:
-  /// This member is defined by this class, but is not used for
-  /// anything.  Subclasses can use it to hold whatever state they find useful.
-  /// This field is initialized to zero by the ctor.
-  uint16_t SubclassData : 15;
+  class SDNodeBitfields {
+    friend class SDNode;
+    friend class MemIntrinsicSDNode;
+
+    bool hasDebugValue() const { return _HasDebugValue; }
+    void setHasDebugValue(bool V) { _HasDebugValue = V; }
+    bool isMemIntrinsic() const { return _IsMemIntrinsic; }
+    void setIsMemIntrinsic(bool V) { _IsMemIntrinsic = V; }
+
+    // Prefixed with "_" so you're not tempted to use these directly.  Use the
+    // accessors above.
+    uint16_t _HasDebugValue : 1;
+    uint16_t _IsMemIntrinsic : 1;
+  };
+  enum { NumSDNodeBits = 2 };
+
+  class ConstantSDNodeBitfields {
+    friend class ConstantSDNode;
+
+    uint16_t : NumSDNodeBits;
+
+    bool isOpaque() const { return _IsOpaque; }
+    void setIsOpaque(bool V) { _IsOpaque = V; }
+
+    uint16_t _IsOpaque : 1;
+  };
+
+  class MemSDNodeBitfields {
+    friend class MemSDNode;
+    friend class MemIntrinsicSDNode;
+    friend class AtomicSDNode;
+
+    uint16_t : NumSDNodeBits;
+
+    bool isVolatile() const { return _IsVolatile; }
+    void setIsVolatile(bool V) { _IsVolatile = V; }
+    bool isNonTemporal() const { return _IsNonTemporal; }
+    void setIsNonTemporal(bool V) { _IsNonTemporal = V; }
+    bool isInvariant() const { return _IsInvariant; }
+    void setIsInvariant(bool V) { _IsInvariant = V; }
+
+    SynchronizationScope synchScope() const {
+      return static_cast<SynchronizationScope>(_SynchScope);
+    }
+    void setSynchScope(SynchronizationScope V) {
+      _SynchScope = static_cast<uint16_t>(V);
+      assert(_SynchScope == V && "Value truncated");
+    }
+
+    AtomicOrdering ordering() const {
+      return static_cast<AtomicOrdering>(_Ordering);
+    }
+    void setOrdering(AtomicOrdering V) {
+      _Ordering = static_cast<uint16_t>(V);
+      assert(_Ordering == static_cast<uint16_t>(V) && "Value truncated");
+    }
+
+    uint16_t _IsVolatile : 1;
+    uint16_t _IsNonTemporal : 1;
+    uint16_t _IsInvariant : 1;
+    uint16_t _SynchScope : 1; // enum SynchronizationScope
+    uint16_t _Ordering : 4;   // enum AtomicOrdering
+  };
+  enum { NumMemSDNodeBits = NumSDNodeBits + 8 };
+
+  class LSBaseSDNodeBitfields {
+    friend class LSBaseSDNode;
+    uint16_t : NumMemSDNodeBits;
+
+    ISD::MemIndexedMode addressingMode() const {
+      return static_cast<ISD::MemIndexedMode>(_AddressingMode);
+    }
+    void setAddressingMode(ISD::MemIndexedMode V) {
+      _AddressingMode = V;
+      assert(_AddressingMode == V && "Value truncated");
+    }
+
+    uint16_t _AddressingMode : 3; // enum ISD::MemIndexedMode
+  };
+  enum { NumLSBaseSDNodeBits = NumMemSDNodeBits + 3 };
+
+  class LoadSDNodeBitfields {
+    friend class LoadSDNode;
+    friend class MaskedLoadSDNode;
+
+    uint16_t : NumLSBaseSDNodeBits;
+
+    ISD::LoadExtType extTy() const {
+      return static_cast<ISD::LoadExtType>(_ExtTy);
+    }
+    void setExtTy(ISD::LoadExtType V) {
+      _ExtTy = static_cast<uint16_t>(V);
+      assert(_ExtTy == V && "Value truncated");
+    }
+
+    uint16_t _ExtTy : 2; // enum ISD::LoadExtType
+  };
+
+  class StoreSDNodeBitfields {
+    friend class StoreSDNode;
+    friend class MaskedStoreSDNode;
+
+    uint16_t : NumLSBaseSDNodeBits;
+
+    bool isTruncating() const { return _IsTruncating; }
+    void setIsTruncating(bool V) { _IsTruncating = V; }
+
+    uint16_t _IsTruncating : 1;
+  };
+
+  union {
+    SDNodeBitfields SDNodeBits;
+    ConstantSDNodeBitfields ConstantSDNodeBits;
+    MemSDNodeBitfields MemSDNodeBits;
+    LSBaseSDNodeBitfields LSBaseSDNodeBits;
+    LoadSDNodeBitfields LoadSDNodeBits;
+    StoreSDNodeBitfields StoreSDNodeBits;
+  };
 
 private:
   /// Unique id per SDNode in the DAG.
   int NodeId;
 
   /// The values that are used by this operation.
   SDUse *OperandList;
 
public:
 
   /// Test if this node is a memory intrinsic (with valid pointer information).
   /// INTRINSIC_W_CHAIN and INTRINSIC_VOID nodes are sometimes created for
   /// non-memory intrinsics (with chains) that are not really instances of
   /// MemSDNode. For such nodes, we need some extra state to determine the
   /// proper classof relationship.
   bool isMemIntrinsic() const {
     return (NodeType == ISD::INTRINSIC_W_CHAIN ||
-            NodeType == ISD::INTRINSIC_VOID) && ((SubclassData >> 13) & 1);
+            NodeType == ISD::INTRINSIC_VOID) &&
+           SDNodeBits.isMemIntrinsic();
   }
 
   /// Test if this node has a post-isel opcode, directly
   /// corresponding to a MachineInstr opcode.
   bool isMachineOpcode() const { return NodeType < 0; }
 
   /// This may only be called if isMachineOpcode returns
   /// true. It returns the MachineInstr opcode value that the node's opcode
   /// corresponds to.
   unsigned getMachineOpcode() const {
     assert(isMachineOpcode() && "Not a MachineInstr opcode!");
     return ~NodeType;
   }
 
-  /// Get this bit.
-  bool getHasDebugValue() const { return HasDebugValue; }
-
-  /// Set this bit.
-  void setHasDebugValue(bool b) { HasDebugValue = b; }
+  bool getHasDebugValue() const { return SDNodeBits.hasDebugValue(); }
+  void setHasDebugValue(bool b) { SDNodeBits.setHasDebugValue(b); }
 
   /// Return true if there are no uses of this node.
   bool use_empty() const { return UseList == nullptr; }
 
   /// Return true if there is exactly one use of this node.
   bool hasOneUse() const {
     return !use_empty() && std::next(use_begin()) == use_end();
   }
static SDVTList getSDVTList(EVT VT) {
     return Ret;
   }
 
   /// Create an SDNode.
   ///
   /// SDNodes are created without any operands, and never own the operand
   /// storage. To add operands, see SelectionDAG::createOperands.
   SDNode(unsigned Opc, unsigned Order, DebugLoc dl, SDVTList VTs)
-      : NodeType(Opc), HasDebugValue(false), SubclassData(0), NodeId(-1),
-        OperandList(nullptr), ValueList(VTs.VTs), UseList(nullptr),
-        NumOperands(0), NumValues(VTs.NumVTs), IROrder(Order),
+      : NodeType(Opc), NodeId(-1), OperandList(nullptr), ValueList(VTs.VTs),
+        UseList(nullptr), NumOperands(0), NumValues(VTs.NumVTs), IROrder(Order),
         debugLoc(std::move(dl)) {
+    memset(&SDNodeBits, 0, sizeof(SDNodeBits));
     assert(debugLoc.hasTrivialDestructor() && "Expected trivial destructor");
     assert(NumValues == VTs.NumVTs &&
            "NumValues wasn't wide enough for its operands!");
   }
 
   /// Release the operands and set this node to have zero operands.
   void DropOperands();
 };
public:
   unsigned getAlignment() const {
     return MMO->getAlignment();
   }
 
   /// Return the SubclassData value, which contains an
   /// encoding of the volatile flag, as well as bits used by subclasses. This
   /// function should only be used to compute a FoldingSetNodeID value.
   unsigned getRawSubclassData() const {
-    return SubclassData;
+    uint16_t Data;
+    memcpy(&Data, &SDNodeBits, sizeof(SDNodeBits));
+    static_assert(sizeof(SDNodeBits) == sizeof(uint16_t),
+                  "SDNodeBits field too small?");
+    return Data;
   }
 
   // We access subclass data here so that we can check consistency
   // with MachineMemOperand information.
-  bool isVolatile() const { return (SubclassData >> 5) & 1; }
-  bool isNonTemporal() const { return (SubclassData >> 6) & 1; }
-  bool isInvariant() const { return (SubclassData >> 7) & 1; }
+  bool isVolatile() const { return MemSDNodeBits.isVolatile(); }
+  bool isNonTemporal() const { return MemSDNodeBits.isNonTemporal(); }
+  bool isInvariant() const { return MemSDNodeBits.isInvariant(); }
 
-  AtomicOrdering getOrdering() const {
-    return AtomicOrdering((SubclassData >> 8) & 15);
-  }
+  AtomicOrdering getOrdering() const { return MemSDNodeBits.ordering(); }
   SynchronizationScope getSynchScope() const {
-    return SynchronizationScope((SubclassData >> 12) & 1);
+    return MemSDNodeBits.synchScope();
   }
 
   // Returns the offset from the location of the access.
   int64_t getSrcValueOffset() const { return MMO->getOffset(); }
 
   /// Returns the AA info that describes the dereference.
   AAMDNodes getAAInfo() const { return MMO->getAAInfo(); }
 

 class AtomicSDNode : public MemSDNode {
   /// For cmpxchg instructions, the ordering requirements when a store does not
   /// occur.
   AtomicOrdering FailureOrdering;
 
   void InitAtomic(AtomicOrdering SuccessOrdering,
                   AtomicOrdering FailureOrdering,
                   SynchronizationScope SynchScope) {
-    // This must match encodeMemSDNodeFlags() in SelectionDAG.cpp.
-    assert((AtomicOrdering)((unsigned)SuccessOrdering & 15) ==
-               SuccessOrdering &&
-           "Ordering may not require more than 4 bits!");
-    assert((AtomicOrdering)((unsigned)FailureOrdering & 15) ==
-               FailureOrdering &&
-           "Ordering may not require more than 4 bits!");
-    assert((SynchScope & 1) == SynchScope &&
-           "SynchScope may not require more than 1 bit!");
-    SubclassData |= (unsigned)SuccessOrdering << 8;
-    SubclassData |= SynchScope << 12;
+    MemSDNodeBits.setOrdering(SuccessOrdering);
+    MemSDNodeBits.setSynchScope(SynchScope);
     this->FailureOrdering = FailureOrdering;
-    assert(getSuccessOrdering() == SuccessOrdering &&
-           "Ordering encoding error!");
-    assert(getFailureOrdering() == FailureOrdering &&
-           "Ordering encoding error!");
-    assert(getSynchScope() == SynchScope && "Synch-scope encoding error!");
   }
 
 public:
   AtomicSDNode(unsigned Opc, unsigned Order, const DebugLoc &dl, SDVTList VTL,
                EVT MemVT, MachineMemOperand *MMO,
                AtomicOrdering SuccessOrdering, AtomicOrdering FailureOrdering,
                SynchronizationScope SynchScope)
       : MemSDNode(Opc, Order, dl, VTL, MemVT, MMO) {
     InitAtomic(SuccessOrdering, FailureOrdering, SynchScope);
   }

 /// memory and need an associated MachineMemOperand. Its opcode may be
 /// INTRINSIC_VOID, INTRINSIC_W_CHAIN, PREFETCH, or a target-specific opcode
 /// with a value not less than FIRST_TARGET_MEMORY_OPCODE.
 class MemIntrinsicSDNode : public MemSDNode {
 public:
   MemIntrinsicSDNode(unsigned Opc, unsigned Order, const DebugLoc &dl,
                      SDVTList VTs, EVT MemoryVT, MachineMemOperand *MMO)
       : MemSDNode(Opc, Order, dl, VTs, MemoryVT, MMO) {
-    SubclassData |= 1u << 13;
+    SDNodeBits.setIsMemIntrinsic(true);
   }
 
   // Methods to support isa and dyn_cast
   static bool classof(const SDNode *N) {
     // We lower some target intrinsics to their target opcode
     // early a node with a target opcode can be of this class
     return N->isMemIntrinsic()             ||
            N->getOpcode() == ISD::PREFETCH ||

 class ConstantSDNode : public SDNode {
   const ConstantInt *Value;
   friend class SelectionDAG;
   ConstantSDNode(bool isTarget, bool isOpaque, const ConstantInt *val,
                  const DebugLoc &DL, EVT VT)
       : SDNode(isTarget ? ISD::TargetConstant : ISD::Constant, 0, DL,
                getSDVTList(VT)),
         Value(val) {
-    SubclassData |= (uint16_t)isOpaque;
+    ConstantSDNodeBits.setIsOpaque(isOpaque);
   }
 public:
 
   const ConstantInt *getConstantIntValue() const { return Value; }
   const APInt &getAPIntValue() const { return Value->getValue(); }
   uint64_t getZExtValue() const { return Value->getZExtValue(); }
   int64_t getSExtValue() const { return Value->getSExtValue(); }
 
   bool isOne() const { return Value->isOne(); }
   bool isNullValue() const { return Value->isNullValue(); }
   bool isAllOnesValue() const { return Value->isAllOnesValue(); }
 
-  bool isOpaque() const { return SubclassData & 1; }
+  bool isOpaque() const { return ConstantSDNodeBits.isOpaque(); }
 
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::Constant ||
            N->getOpcode() == ISD::TargetConstant;
   }
 };
 
 class ConstantFPSDNode : public SDNode {

 
 /// Base class for LoadSDNode and StoreSDNode
 class LSBaseSDNode : public MemSDNode {
 public:
   LSBaseSDNode(ISD::NodeType NodeTy, unsigned Order, const DebugLoc &dl,
                SDVTList VTs, ISD::MemIndexedMode AM, EVT MemVT,
                MachineMemOperand *MMO)
       : MemSDNode(NodeTy, Order, dl, VTs, MemVT, MMO) {
-    SubclassData |= AM << 2;
-    assert(getAddressingMode() == AM && "MemIndexedMode encoding error!");
+    LSBaseSDNodeBits.setAddressingMode(AM);
   }
 
   const SDValue &getOffset() const {
     return getOperand(getOpcode() == ISD::LOAD ? 2 : 3);
   }
 
   /// Return the addressing mode for this load or store:
   /// unindexed, pre-inc, pre-dec, post-inc, or post-dec.
   ISD::MemIndexedMode getAddressingMode() const {
-    return ISD::MemIndexedMode((SubclassData >> 2) & 7);
+    return LSBaseSDNodeBits.addressingMode();
   }
 
   /// Return true if this is a pre/post inc/dec load/store.
   bool isIndexed() const { return getAddressingMode() != ISD::UNINDEXED; }
 
   /// Return true if this is NOT a pre/post inc/dec load/store.
   bool isUnindexed() const { return getAddressingMode() == ISD::UNINDEXED; }
 
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::LOAD ||
            N->getOpcode() == ISD::STORE;
   }
 };
 
 /// This class is used to represent ISD::LOAD nodes.
 class LoadSDNode : public LSBaseSDNode {
   friend class SelectionDAG;
   LoadSDNode(unsigned Order, const DebugLoc &dl, SDVTList VTs,
              ISD::MemIndexedMode AM, ISD::LoadExtType ETy, EVT MemVT,
              MachineMemOperand *MMO)
       : LSBaseSDNode(ISD::LOAD, Order, dl, VTs, AM, MemVT, MMO) {
-    SubclassData |= (unsigned short)ETy;
-    assert(getExtensionType() == ETy && "LoadExtType encoding error!");
-    assert(readMem() && "Load MachineMemOperand is not a load!");
-    assert(!writeMem() && "Load MachineMemOperand is a store!");
+    LoadSDNodeBits.setExtTy(ETy);
   }
 public:
 
   /// Return whether this is a plain node,
   /// or one of the varieties of value-extending loads.
-  ISD::LoadExtType getExtensionType() const {
-    return ISD::LoadExtType(SubclassData & 3);
-  }
+  ISD::LoadExtType getExtensionType() const { return LoadSDNodeBits.extTy(); }
 
   const SDValue &getBasePtr() const { return getOperand(1); }
   const SDValue &getOffset() const { return getOperand(2); }
 
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::LOAD;
   }
 };
 
 /// This class is used to represent ISD::STORE nodes.
 class StoreSDNode : public LSBaseSDNode {
   friend class SelectionDAG;
   StoreSDNode(unsigned Order, const DebugLoc &dl, SDVTList VTs,
               ISD::MemIndexedMode AM, bool isTrunc, EVT MemVT,
               MachineMemOperand *MMO)
       : LSBaseSDNode(ISD::STORE, Order, dl, VTs, AM, MemVT, MMO) {
-    SubclassData |= (unsigned short)isTrunc;
-    assert(isTruncatingStore() == isTrunc && "isTrunc encoding error!");
-    assert(!readMem() && "Store MachineMemOperand is a load!");
-    assert(writeMem() && "Store MachineMemOperand is not a store!");
+    StoreSDNodeBits.setIsTruncating(isTrunc);
   }
 public:
 
   /// Return true if the op does a truncation before store.
   /// For integers this is the same as doing a TRUNCATE and storing the result.
   /// For floats, it is the same as doing an FP_ROUND and storing the result.
-  bool isTruncatingStore() const { return SubclassData & 1; }
+  bool isTruncatingStore() const { return StoreSDNodeBits.isTruncating(); }
 
   const SDValue &getValue() const { return getOperand(1); }
   const SDValue &getBasePtr() const { return getOperand(2); }
   const SDValue &getOffset() const { return getOperand(3); }
 
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::STORE;
   }

 
 /// This class is used to represent an MLOAD node
 class MaskedLoadSDNode : public MaskedLoadStoreSDNode {
 public:
   friend class SelectionDAG;
   MaskedLoadSDNode(unsigned Order, const DebugLoc &dl, SDVTList VTs,
                    ISD::LoadExtType ETy, EVT MemVT, MachineMemOperand *MMO)
       : MaskedLoadStoreSDNode(ISD::MLOAD, Order, dl, VTs, MemVT, MMO) {
-    SubclassData |= (unsigned short)ETy;
+    LoadSDNodeBits.setExtTy(ETy);
   }
 
-  ISD::LoadExtType getExtensionType() const {
-    return ISD::LoadExtType(SubclassData & 3);
-  }
+  ISD::LoadExtType getExtensionType() const { return LoadSDNodeBits.extTy(); }
+
   const SDValue &getSrc0() const { return getOperand(3); }
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::MLOAD;
   }
 };
 
 /// This class is used to represent an MSTORE node
 class MaskedStoreSDNode : public MaskedLoadStoreSDNode {
 
 public:
   friend class SelectionDAG;
   MaskedStoreSDNode(unsigned Order, const DebugLoc &dl, SDVTList VTs,
                     bool isTrunc, EVT MemVT, MachineMemOperand *MMO)
       : MaskedLoadStoreSDNode(ISD::MSTORE, Order, dl, VTs, MemVT, MMO) {
-    SubclassData |= (unsigned short)isTrunc;
+    StoreSDNodeBits.setIsTruncating(isTrunc);
   }
   /// Return true if the op does a truncation before store.
   /// For integers this is the same as doing a TRUNCATE and storing the result.
   /// For floats, it is the same as doing an FP_ROUND and storing the result.
-  bool isTruncatingStore() const { return SubclassData & 1; }
+  bool isTruncatingStore() const { return StoreSDNodeBits.isTruncating(); }
 
   const SDValue &getValue() const { return getOperand(3); }
 
   static bool classof(const SDNode *N) {
     return N->getOpcode() == ISD::MSTORE;
   }
 };
 

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

static void AddNodeIDNode(FoldingSetNodeID &ID, const SDNode *N) {
   AddNodeIDValueTypes(ID, N->getVTList());
   // Add the operand info.
   AddNodeIDOperands(ID, N->ops());
 
   // Handle SDNode leafs with special info.
   AddNodeIDCustom(ID, N);
 }
 
-/// encodeMemSDNodeFlags - Generic routine for computing a value for use in
-/// the CSE map that carries volatility, temporalness, indexing mode, and
-/// extension/truncation information.
-///
-static inline unsigned
-encodeMemSDNodeFlags(int ConvType, ISD::MemIndexedMode AM, bool isVolatile,
-                     bool isNonTemporal, bool isInvariant) {
-  assert((ConvType & 3) == ConvType &&
-         "ConvType may not require more than 2 bits!");
-  assert((AM & 7) == AM &&
-         "AM may not require more than 3 bits!");
-  return ConvType |
-         (AM << 2) |
-         (isVolatile << 5) |
-         (isNonTemporal << 6) |
-         (isInvariant << 7);
-}
-
 //===----------------------------------------------------------------------===//
 //                              SelectionDAG Class
 //===----------------------------------------------------------------------===//
 
 /// doNotCSE - Return true if CSE should not be performed for this node.
 static bool doNotCSE(SDNode *N) {
   if (N->getValueType(0) == MVT::Glue)
     return true; // Never CSE anything that produces a flag.
SDValue SelectionDAG::getLoad(ISD::MemIndexedMode AM, ISD::LoadExtType ExtType,
   assert((Indexed || Offset.isUndef()) && "Unindexed load with an offset!");
 
   SDVTList VTs = Indexed ?
     getVTList(VT, Ptr.getValueType(), MVT::Other) : getVTList(VT, MVT::Other);
   SDValue Ops[] = { Chain, Ptr, Offset };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::LOAD, VTs, Ops);
   ID.AddInteger(MemVT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(ExtType, AM, MMO->isVolatile(),
-                                     MMO->isNonTemporal(),
-                                     MMO->isInvariant()));
+  // The compiler can reduce this expression to a constant iff we pass an empty
+  // DebugLoc.  Thankfully, the debug location doesn't have any bearing on the
+  // subclass data.
+  ID.AddInteger(
+      LoadSDNode(dl.getIROrder(), DebugLoc(), VTs, AM, ExtType, MemVT, MMO)
+          .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<LoadSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<LoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs, AM,
                                   ExtType, MemVT, MMO);
   createOperands(N, Ops);
assert(Chain.getValueType() == MVT::Other &&
         "Invalid chain type");
   EVT VT = Val.getValueType();
   SDVTList VTs = getVTList(MVT::Other);
   SDValue Undef = getUNDEF(Ptr.getValueType());
   SDValue Ops[] = { Chain, Val, Ptr, Undef };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::STORE, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
-                                     MMO->isNonTemporal(), MMO->isInvariant()));
+  ID.AddInteger(StoreSDNode(dl.getIROrder(), DebugLoc(), VTs, ISD::UNINDEXED,
+                            false, VT, MMO)
+                    .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<StoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<StoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                    ISD::UNINDEXED, false, VT, MMO);
assert((!VT.isVector() ||
          "Cannot use trunc store to change the number of vector elements!");
 
   SDVTList VTs = getVTList(MVT::Other);
   SDValue Undef = getUNDEF(Ptr.getValueType());
   SDValue Ops[] = { Chain, Val, Ptr, Undef };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::STORE, VTs, Ops);
   ID.AddInteger(SVT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(true, ISD::UNINDEXED, MMO->isVolatile(),
-                                     MMO->isNonTemporal(), MMO->isInvariant()));
+  ID.AddInteger(StoreSDNode(dl.getIROrder(), DebugLoc(), VTs, ISD::UNINDEXED,
+                            true, SVT, MMO)
+                    .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<StoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<StoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                    ISD::UNINDEXED, true, SVT, MMO);
SDValue SelectionDAG::getMaskedLoad(EVT VT, const SDLoc &dl, SDValue Chain,
                                     EVT MemVT, MachineMemOperand *MMO,
                                     ISD::LoadExtType ExtTy) {
 
   SDVTList VTs = getVTList(VT, MVT::Other);
   SDValue Ops[] = { Chain, Ptr, Mask, Src0 };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MLOAD, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(ExtTy, ISD::UNINDEXED,
-                                     MMO->isVolatile(),
-                                     MMO->isNonTemporal(),
-                                     MMO->isInvariant()));
+  ID.AddInteger(
+      MaskedLoadSDNode(dl.getIROrder(), DebugLoc(), VTs, ExtTy, MemVT, MMO)
+          .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedLoadSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedLoadSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                         ExtTy, MemVT, MMO);
SDValue SelectionDAG::getMaskedStore(SDValue Chain, const SDLoc &dl,
   assert(Chain.getValueType() == MVT::Other &&
         "Invalid chain type");
   EVT VT = Val.getValueType();
   SDVTList VTs = getVTList(MVT::Other);
   SDValue Ops[] = { Chain, Ptr, Mask, Val };
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MSTORE, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
-                                     MMO->isNonTemporal(), MMO->isInvariant()));
+  ID.AddInteger(
+      MaskedStoreSDNode(dl.getIROrder(), DebugLoc(), VTs, isTrunc, MemVT, MMO)
+          .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedStoreSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedStoreSDNode>(dl.getIROrder(), dl.getDebugLoc(), VTs,
                                          isTrunc, MemVT, MMO);
   createOperands(N, Ops);
 
   CSEMap.InsertNode(N, IP);
   InsertNode(N);
   return SDValue(N, 0);
 }
 
 SDValue SelectionDAG::getMaskedGather(SDVTList VTs, EVT VT, const SDLoc &dl,
                                       ArrayRef<SDValue> Ops,
                                       MachineMemOperand *MMO) {
   assert(Ops.size() == 5 && "Incompatible number of operands");
 
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MGATHER, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(ISD::NON_EXTLOAD, ISD::UNINDEXED,
-                                     MMO->isVolatile(),
-                                     MMO->isNonTemporal(),
-                                     MMO->isInvariant()));
+  ID.AddInteger(MaskedGatherSDNode(dl.getIROrder(), DebugLoc(), VTs, VT, MMO)
+                    .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedGatherSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
 
   auto *N = newSDNode<MaskedGatherSDNode>(dl.getIROrder(), dl.getDebugLoc(),

 SDValue SelectionDAG::getMaskedScatter(SDVTList VTs, EVT VT, const SDLoc &dl,
                                        ArrayRef<SDValue> Ops,
                                        MachineMemOperand *MMO) {
   assert(Ops.size() == 5 && "Incompatible number of operands");
 
   FoldingSetNodeID ID;
   AddNodeIDNode(ID, ISD::MSCATTER, VTs, Ops);
   ID.AddInteger(VT.getRawBits());
-  ID.AddInteger(encodeMemSDNodeFlags(false, ISD::UNINDEXED, MMO->isVolatile(),
-                                     MMO->isNonTemporal(),
-                                     MMO->isInvariant()));
+  ID.AddInteger(MaskedScatterSDNode(dl.getIROrder(), DebugLoc(), VTs, VT, MMO)
+                    .getRawSubclassData());
   ID.AddInteger(MMO->getPointerInfo().getAddrSpace());
   void *IP = nullptr;
   if (SDNode *E = FindNodeOrInsertPos(ID, dl, IP)) {
     cast<MaskedScatterSDNode>(E)->refineAlignment(MMO);
     return SDValue(E, 0);
   }
   auto *N = newSDNode<MaskedScatterSDNode>(dl.getIROrder(), dl.getDebugLoc(),
                                            VTs, VT, MMO);
AddrSpaceCastSDNode::AddrSpaceCastSDNode(unsigned Order, const DebugLoc &dl,
                                          EVT VT, unsigned SrcAS,
                                          unsigned DestAS)
     : SDNode(ISD::ADDRSPACECAST, Order, dl, getSDVTList(VT)),
       SrcAddrSpace(SrcAS), DestAddrSpace(DestAS) {}
 
 MemSDNode::MemSDNode(unsigned Opc, unsigned Order, const DebugLoc &dl,
                      SDVTList VTs, EVT memvt, MachineMemOperand *mmo)
     : SDNode(Opc, Order, dl, VTs), MemoryVT(memvt), MMO(mmo) {
-  SubclassData = encodeMemSDNodeFlags(0, ISD::UNINDEXED, MMO->isVolatile(),
-                                      MMO->isNonTemporal(), MMO->isInvariant());
-  assert(isVolatile() == MMO->isVolatile() && "Volatile encoding error!");
-  assert(isNonTemporal() == MMO->isNonTemporal() &&
-         "Non-temporal encoding error!");
+  MemSDNodeBits.setIsVolatile(MMO->isVolatile());
+  MemSDNodeBits.setIsNonTemporal(MMO->isNonTemporal());
+  MemSDNodeBits.setIsInvariant(MMO->isInvariant());
+
   // We check here that the size of the memory operand fits within the size of
   // the MMO. This is because the MMO might indicate only a possible address
   // range instead of specifying the affected memory addresses precisely.
   assert(memvt.getStoreSize() <= MMO->getSize() && "Size mismatch!");
 }
 
 /// Profile - Gather unique data for the node.
 ///