Switch all of the uses of my InsertDAGNode helper to follow the exact

same pattern. We already had this pattern is a few places, but others
tried to make a rough approximation of an actual DAG structure. As not
everywhere went to this trouble, nothing could rely on this being done.
In fact, I've checked all references to these node Ids, and the ones
that are using the topo-sort properties are actually satisfied with
a strict-weak-ordering. The requirement appears to be that Use >= Def.

I've added a big blurb of comments to this bit of the transform to
clarify why the order is so important for the next reader of the code.

I'm starting with this change as it is very small, and trivially
reverted if something breaks or the >= above really does need to be >.
If that proves the case, we can hide the problem by reverting this
patch, but the problem exists elsewhere as well, and so a more
comprehensive solution will be needed.

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

diff --git a/lib/Target/X86/X86ISelDAGToDAG.cpp b/lib/Target/X86/X86ISelDAGToDAG.cpp
index 441267a2a4df0b532fe217b72e404e238f0648fa..ffb1522c6f65385e168c4c2172aadf8e7da74e71 100644 (file)
--- a/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -764,12 +764,16 @@ static bool FoldMaskAndShiftToExtract(SelectionDAG &DAG, SDValue N,
   SDValue ShlCount = DAG.getConstant(ScaleLog, MVT::i8);
   SDValue Shl = DAG.getNode(ISD::SHL, DL, VT, And, ShlCount);
 
-  // Insert the new nodes into the topological ordering.
-  InsertDAGNode(DAG, X, Eight);
-  InsertDAGNode(DAG, X, NewMask);
-  InsertDAGNode(DAG, Shift, Srl);
+  // Insert the new nodes into the topological ordering. We must do this in
+  // a valid topological ordering as nothing is going to go back and re-sort
+  // these nodes. We continually insert before 'N' in sequence as this is
+  // essentially a pre-flattened and pre-sorted sequence of nodes. There is no
+  // hierarchy left to express.
+  InsertDAGNode(DAG, N, Eight);
+  InsertDAGNode(DAG, N, Srl);
+  InsertDAGNode(DAG, N, NewMask);
   InsertDAGNode(DAG, N, And);
-  InsertDAGNode(DAG, X, ShlCount);
+  InsertDAGNode(DAG, N, ShlCount);
   InsertDAGNode(DAG, N, Shl);
   DAG.ReplaceAllUsesWith(N, Shl);
   AM.IndexReg = And;
@@ -805,9 +809,13 @@ static bool FoldMaskedShiftToScaledMask(SelectionDAG &DAG, SDValue N,
   SDValue NewAnd = DAG.getNode(ISD::AND, DL, VT, X, NewMask);
   SDValue NewShift = DAG.getNode(ISD::SHL, DL, VT, NewAnd, Shift.getOperand(1));
 
-  // Insert the new nodes into the topological ordering.
-  InsertDAGNode(DAG, X, NewMask);
-  InsertDAGNode(DAG, Shift, NewAnd);
+  // Insert the new nodes into the topological ordering. We must do this in
+  // a valid topological ordering as nothing is going to go back and re-sort
+  // these nodes. We continually insert before 'N' in sequence as this is
+  // essentially a pre-flattened and pre-sorted sequence of nodes. There is no
+  // hierarchy left to express.
+  InsertDAGNode(DAG, N, NewMask);
+  InsertDAGNode(DAG, N, NewAnd);
   InsertDAGNode(DAG, N, NewShift);
   DAG.ReplaceAllUsesWith(N, NewShift);
 
@@ -907,6 +915,12 @@ static bool FoldMaskAndShiftToScale(SelectionDAG &DAG, SDValue N,
   SDValue NewSRL = DAG.getNode(ISD::SRL, DL, VT, X, NewSRLAmt);
   SDValue NewSHLAmt = DAG.getConstant(AMShiftAmt, MVT::i8);
   SDValue NewSHL = DAG.getNode(ISD::SHL, DL, VT, NewSRL, NewSHLAmt);
+
+  // Insert the new nodes into the topological ordering. We must do this in
+  // a valid topological ordering as nothing is going to go back and re-sort
+  // these nodes. We continually insert before 'N' in sequence as this is
+  // essentially a pre-flattened and pre-sorted sequence of nodes. There is no
+  // hierarchy left to express.
   InsertDAGNode(DAG, N, NewSRLAmt);
   InsertDAGNode(DAG, N, NewSRL);
   InsertDAGNode(DAG, N, NewSHLAmt);