[SDAG] Start plumbing an assert into SDValues that we don't form one
authorChandler Carruth <chandlerc@gmail.com>
Fri, 25 Jul 2014 07:23:23 +0000 (07:23 +0000)
committerChandler Carruth <chandlerc@gmail.com>
Fri, 25 Jul 2014 07:23:23 +0000 (07:23 +0000)
with a result number outside the range of results for the node.

I don't know how we managed to not really check this very basic
invariant for so long, but the code is *very* broken at this point.
I have over 270 test failures with the assert enabled. I'm committing it
disabled so that others can join in the cleanup effort and reproduce the
issues. I've also included one of the obvious fixes that I already
found. More fixes to come.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@213926 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/SelectionDAGNodes.h
lib/CodeGen/SelectionDAG/DAGCombiner.cpp

index 0aa96cdf0cb1df90b90830e20124b894a475eeea..f93f8e41e62c23206b2a75a2005f6ffd35e1b532 100644 (file)
@@ -117,11 +117,13 @@ namespace ISD {
 /// of information is represented with the SDValue value type.
 ///
 class SDValue {
+  friend struct DenseMapInfo<SDValue>;
+
   SDNode *Node;       // The node defining the value we are using.
   unsigned ResNo;     // Which return value of the node we are using.
 public:
   SDValue() : Node(nullptr), ResNo(0) {}
-  SDValue(SDNode *node, unsigned resno) : Node(node), ResNo(resno) {}
+  SDValue(SDNode *node, unsigned resno);
 
   /// get the index which selects a specific result in the SDNode
   unsigned getResNo() const { return ResNo; }
@@ -208,10 +210,14 @@ public:
 
 template<> struct DenseMapInfo<SDValue> {
   static inline SDValue getEmptyKey() {
-    return SDValue((SDNode*)-1, -1U);
+    SDValue V;
+    V.ResNo = -1U;
+    return V;
   }
   static inline SDValue getTombstoneKey() {
-    return SDValue((SDNode*)-1, 0);
+    SDValue V;
+    V.ResNo = -2U;
+    return V;
   }
   static unsigned getHashValue(const SDValue &Val) {
     return ((unsigned)((uintptr_t)Val.getNode() >> 4) ^
@@ -877,6 +883,18 @@ public:
 
 // Define inline functions from the SDValue class.
 
+inline SDValue::SDValue(SDNode *node, unsigned resno)
+    : Node(node), ResNo(resno) {
+// This is currently disabled because it fires pretty widely, but I wanted to
+// commit it so others could help reproduce and aid in the cleanup. It will get
+// enabled ASAP.
+#if 0
+  assert((!Node || ResNo < Node->getNumValues()) &&
+         "Invalid result number for the given node!");
+#endif
+  assert(ResNo < -2U && "Cannot use result numbers reserved for DenseMaps.");
+}
+
 inline unsigned SDValue::getOpcode() const {
   return Node->getOpcode();
 }
index 379ea7f7f09d462d3339e629c6e8b942a7658878..a9ba5b87bb90e334c898f1de1997981a1b60ce61 100644 (file)
@@ -9088,7 +9088,7 @@ bool DAGCombiner::MergeConsecutiveStores(StoreSDNode* St) {
     return false;
 
   // Only look at ends of store sequences.
-  SDValue Chain = SDValue(St, 1);
+  SDValue Chain = SDValue(St, 0);
   if (Chain->hasOneUse() && Chain->use_begin()->getOpcode() == ISD::STORE)
     return false;