From: Chandler Carruth Date: Thu, 12 Jan 2012 01:34:44 +0000 (+0000) Subject: Switch all of the uses of my InsertDAGNode helper to follow the exact X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=0fe9a92b33269d005253a2ed47d55dba48929c48;p=oota-llvm.git 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 441267a2a4d..ffb1522c6f6 100644 --- 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);