Fix some issues in WalkChainUsers dealing with
authorChris Lattner <sabre@nondot.org>
Tue, 2 Mar 2010 22:20:06 +0000 (22:20 +0000)
committerChris Lattner <sabre@nondot.org>
Tue, 2 Mar 2010 22:20:06 +0000 (22:20 +0000)
CopyToReg/CopyFromReg/INLINEASM.  These are annoying because
they have the same opcode before an after isel.  Fix this by
setting their NodeID to -1 to indicate that they are selected,
just like what automatically happens when selecting things that
end up being machine nodes.

With that done, give IsLegalToFold a new flag that causes it to
ignore chains.  This lets the HandleMergeInputChains routine be
the one place that validates chains after a match is successful,
enabling the new hotness in chain processing.  This smarter
chain processing eliminates the need for "PreprocessRMW" in the
X86 and MSP430 backends and enables MSP to start matching it's
multiple mem operand instructions more aggressively.

I currently #if out the dead code in the X86 backend and MSP
backend, I'll remove it for real in a follow-on patch.

The testcase changes are:
  test/CodeGen/X86/sse3.ll: we generate better code
  test/CodeGen/X86/store_op_load_fold2.ll: PreprocessRMW was
      miscompiling this before, we now generate correct code
      Convert it to filecheck while I'm at it.
  test/CodeGen/MSP430/Inst16mm.ll: Add a testcase for mem/mem
      folding to make anton happy. :)

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

include/llvm/CodeGen/SelectionDAGISel.h
lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
lib/Target/MSP430/MSP430ISelDAGToDAG.cpp
lib/Target/X86/X86ISelDAGToDAG.cpp
test/CodeGen/MSP430/Inst16mm.ll
test/CodeGen/X86/sse3.ll
test/CodeGen/X86/store_op_load_fold2.ll

index af1273c1e062c62c26bce26d7d6c9427402470f6..9ee8604c7897ab725c4b75e1a447b457a0083d0a 100644 (file)
@@ -97,7 +97,8 @@ public:
 
   /// IsLegalToFold - Returns true if the specific operand node N of
   /// U can be folded during instruction selection that starts at Root.
-  virtual bool IsLegalToFold(SDValue N, SDNode *U, SDNode *Root) const;
+  virtual bool IsLegalToFold(SDValue N, SDNode *U, SDNode *Root,
+                             bool IgnoreChains = false) const;
 
   /// CreateTargetHazardRecognizer - Return a newly allocated hazard recognizer
   /// to use for this target when scheduling the DAG.
index ff8ab466d5f6b865483138d54d1bd2a8492428ee..8e88f16d699a0a9366228198178c4b340a966869 100644 (file)
@@ -1376,8 +1376,8 @@ static SDNode *findFlagUse(SDNode *N) {
 /// This function recursively traverses up the operand chain, ignoring
 /// certain nodes.
 static bool findNonImmUse(SDNode *Use, SDNode* Def, SDNode *ImmedUse,
-                          SDNode *Root,
-                          SmallPtrSet<SDNode*, 16> &Visited) {
+                          SDNode *Root, SmallPtrSet<SDNode*, 16> &Visited,
+                          bool IgnoreChains) {
   // The NodeID's are given uniques ID's where a node ID is guaranteed to be
   // greater than all of its (recursive) operands.  If we scan to a point where
   // 'use' is smaller than the node we're scanning for, then we know we will
@@ -1395,6 +1395,10 @@ static bool findNonImmUse(SDNode *Use, SDNode* Def, SDNode *ImmedUse,
     return false;
 
   for (unsigned i = 0, e = Use->getNumOperands(); i != e; ++i) {
+    // Ignore chain uses, they are validated by HandleMergeInputChains.
+    if (Use->getOperand(i).getValueType() == MVT::Other && IgnoreChains)
+      continue;
+    
     SDNode *N = Use->getOperand(i).getNode();
     if (N == Def) {
       if (Use == ImmedUse || Use == Root)
@@ -1404,7 +1408,7 @@ static bool findNonImmUse(SDNode *Use, SDNode* Def, SDNode *ImmedUse,
     }
 
     // Traverse up the operand chain.
-    if (findNonImmUse(N, Def, ImmedUse, Root, Visited))
+    if (findNonImmUse(N, Def, ImmedUse, Root, Visited, IgnoreChains))
       return true;
   }
   return false;
@@ -1419,9 +1423,10 @@ static bool findNonImmUse(SDNode *Use, SDNode* Def, SDNode *ImmedUse,
 /// have one non-chain use, we only need to watch out for load/op/store
 /// and load/op/cmp case where the root (store / cmp) may reach the load via
 /// its chain operand.
-static inline bool isNonImmUse(SDNode *Root, SDNode *Def, SDNode *ImmedUse) {
+static inline bool isNonImmUse(SDNode *Root, SDNode *Def, SDNode *ImmedUse,
+                               bool IgnoreChains) {
   SmallPtrSet<SDNode*, 16> Visited;
-  return findNonImmUse(Root, Def, ImmedUse, Root, Visited);
+  return findNonImmUse(Root, Def, ImmedUse, Root, Visited, IgnoreChains);
 }
 
 /// IsProfitableToFold - Returns true if it's profitable to fold the specific
@@ -1434,7 +1439,8 @@ bool SelectionDAGISel::IsProfitableToFold(SDValue N, SDNode *U,
 
 /// IsLegalToFold - Returns true if the specific operand node N of
 /// U can be folded during instruction selection that starts at Root.
-bool SelectionDAGISel::IsLegalToFold(SDValue N, SDNode *U, SDNode *Root) const {
+bool SelectionDAGISel::IsLegalToFold(SDValue N, SDNode *U, SDNode *Root,
+                                     bool IgnoreChains) const {
   if (OptLevel == CodeGenOpt::None) return false;
 
   // If Root use can somehow reach N through a path that that doesn't contain
@@ -1488,7 +1494,7 @@ bool SelectionDAGISel::IsLegalToFold(SDValue N, SDNode *U, SDNode *Root) const {
     VT = Root->getValueType(Root->getNumValues()-1);
   }
 
-  return !isNonImmUse(Root, N.getNode(), U);
+  return !isNonImmUse(Root, N.getNode(), U, IgnoreChains);
 }
 
 SDNode *SelectionDAGISel::Select_INLINEASM(SDNode *N) {
@@ -1500,6 +1506,7 @@ SDNode *SelectionDAGISel::Select_INLINEASM(SDNode *N) {
   VTs.push_back(MVT::Flag);
   SDValue New = CurDAG->getNode(ISD::INLINEASM, N->getDebugLoc(),
                                 VTs, &Ops[0], Ops.size());
+  New->setNodeId(-1);
   return New.getNode();
 }
 
@@ -1636,11 +1643,17 @@ WalkChainUsers(SDNode *ChainedNode,
     // pattern that we're selecting down into the already selected chunk of the
     // DAG.
     if (User->isMachineOpcode() ||
-        User->getOpcode() == ISD::CopyToReg ||
-        User->getOpcode() == ISD::CopyFromReg ||
-        User->getOpcode() == ISD::INLINEASM ||
         User->getOpcode() == ISD::HANDLENODE)  // Root of the graph.
       continue;
+    
+    if (User->getOpcode() == ISD::CopyToReg ||
+        User->getOpcode() == ISD::CopyFromReg ||
+        User->getOpcode() == ISD::INLINEASM) {
+      // If their node ID got reset to -1 then they've already been selected.
+      // Treat them like a MachineOpcode.
+      if (User->getNodeId() == -1)
+        continue;
+    }
 
     // If we have a TokenFactor, we handle it specially.
     if (User->getOpcode() != ISD::TokenFactor) {
@@ -1876,6 +1889,7 @@ SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
   case ISD::TokenFactor:
   case ISD::CopyFromReg:
   case ISD::CopyToReg:
+    NodeToMatch->setNodeId(-1); // Mark selected.
     return 0;
   case ISD::AssertSext:
   case ISD::AssertZext:
@@ -2172,7 +2186,7 @@ SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
       if (!IsProfitableToFold(N, NodeStack[NodeStack.size()-2].getNode(),
                               NodeToMatch) ||
           !IsLegalToFold(N, NodeStack[NodeStack.size()-2].getNode(),
-                         NodeToMatch))
+                         NodeToMatch, true/*We validate our own chains*/))
         break;
       
       continue;
index 54062a00f0278e94248680516131869c8a0b9553..2ca184e5eafd2d354b26619f57d3af330a494bfd 100644 (file)
@@ -125,7 +125,9 @@ namespace {
     bool MatchWrapper(SDValue N, MSP430ISelAddressMode &AM);
     bool MatchAddressBase(SDValue N, MSP430ISelAddressMode &AM);
 
+#if 0
     bool IsLegalToFold(SDValue N, SDNode *U, SDNode *Root) const;
+#endif
 
     virtual bool
     SelectInlineAsmMemoryOperand(const SDValue &Op, char ConstraintCode,
@@ -323,6 +325,7 @@ SelectInlineAsmMemoryOperand(const SDValue &Op, char ConstraintCode,
   return false;
 }
 
+#if 0
 bool MSP430DAGToDAGISel::IsLegalToFold(SDValue N, SDNode *U,
                                        SDNode *Root) const {
   if (OptLevel == CodeGenOpt::None) return false;
@@ -357,6 +360,7 @@ bool MSP430DAGToDAGISel::IsLegalToFold(SDValue N, SDNode *U,
   // Proceed to 'generic' cycle finder code
   return SelectionDAGISel::IsLegalToFold(N, U, Root);
 }
+#endif
 
 
 /// MoveBelowTokenFactor - Replace TokenFactor operand with load's chain operand
@@ -516,6 +520,7 @@ static bool isRMWLoad(SDValue N, SDValue Chain, SDValue Address,
 /// This allows selection of mem-mem instructions. Yay!
 
 void MSP430DAGToDAGISel::PreprocessForRMW() {
+  return;
   for (SelectionDAG::allnodes_iterator I = CurDAG->allnodes_begin(),
          E = CurDAG->allnodes_end(); I != E; ++I) {
     if (!ISD::isNON_TRUNCStore(I))
index e162f4e4175eade541ebbbcbdba2411feb362f8f..5ec50650b6dfbea5141eed18f1df85acc85c454e 100644 (file)
@@ -467,46 +467,6 @@ static bool isCalleeLoad(SDValue Callee, SDValue &Chain) {
 }
 
 
-/// PreprocessForRMW - Preprocess the DAG to make instruction selection better.
-/// This is only run if not in -O0 mode.
-/// This allows the instruction selector to pick more read-modify-write
-/// instructions. This is a common case:
-///
-///     [Load chain]
-///         ^
-///         |
-///       [Load]
-///       ^    ^
-///       |    |
-///      /      \-
-///     /         |
-/// [TokenFactor] [Op]
-///     ^          ^
-///     |          |
-///      \        /
-///       \      /
-///       [Store]
-///
-/// The fact the store's chain operand != load's chain will prevent the
-/// (store (op (load))) instruction from being selected. We can transform it to:
-///
-///     [Load chain]
-///         ^
-///         |
-///    [TokenFactor]
-///         ^
-///         |
-///       [Load]
-///       ^    ^
-///       |    |
-///       |     \- 
-///       |       | 
-///       |     [Op]
-///       |       ^
-///       |       |
-///       \      /
-///        \    /
-///       [Store]
 void X86DAGToDAGISel::PreprocessForRMW() {
   for (SelectionDAG::allnodes_iterator I = CurDAG->allnodes_begin(),
          E = CurDAG->allnodes_end(); I != E; ++I) {
@@ -538,6 +498,9 @@ void X86DAGToDAGISel::PreprocessForRMW() {
       ++NumLoadMoved;
       continue;
     }
+    
+    continue;
+    
 
     if (!ISD::isNON_TRUNCStore(I))
       continue;
@@ -1415,11 +1378,12 @@ bool X86DAGToDAGISel::TryFoldLoad(SDNode *P, SDValue N,
                                   SDValue &Base, SDValue &Scale,
                                   SDValue &Index, SDValue &Disp,
                                   SDValue &Segment) {
-  if (ISD::isNON_EXTLoad(N.getNode()) &&
-      IsProfitableToFold(N, P, P) &&
-      IsLegalToFold(N, P, P))
-    return SelectAddr(P, N.getOperand(1), Base, Scale, Index, Disp, Segment);
-  return false;
+  if (!ISD::isNON_EXTLoad(N.getNode()) ||
+      !IsProfitableToFold(N, P, P) ||
+      !IsLegalToFold(N, P, P))
+    return false;
+  
+  return SelectAddr(P, N.getOperand(1), Base, Scale, Index, Disp, Segment);
 }
 
 /// getGlobalBaseReg - Return an SDNode that returns the value of
index 510afe373494aa3f51623472247b09f65bb76b17..2337c2c0f241782d9ff09aa577a5db9415b9db4f 100644 (file)
@@ -1,4 +1,4 @@
-; RUN: llc -march=msp430 < %s | FileCheck %s
+; RUN: llc -march=msp430 -combiner-alias-analysis < %s | FileCheck %s
 target datalayout = "e-p:16:8:8-i8:8:8-i16:8:8-i32:8:8"
 target triple = "msp430-generic-generic"
 @foo = common global i16 0, align 2
@@ -52,3 +52,18 @@ define void @xor() nounwind {
        ret void
 }
 
+define i16 @mov2() nounwind {
+entry:
+ %retval = alloca i16                            ; <i16*> [#uses=3]
+ %x = alloca i32, align 2                        ; <i32*> [#uses=1]
+ %y = alloca i32, align 2                        ; <i32*> [#uses=1]
+ store i16 0, i16* %retval
+ %tmp = load i32* %y                             ; <i32> [#uses=1]
+ store i32 %tmp, i32* %x
+ store i16 0, i16* %retval
+ %0 = load i16* %retval                          ; <i16> [#uses=1]
+ ret i16 %0
+; CHECK: mov2:
+; CHECK:       mov.w   0(r1), 4(r1)
+; CHECK:       mov.w   2(r1), 6(r1)
+}
index b2af7c947d98125c4af9e755ae1b2c266a43c14a..921161e4a122b9400f095fd80cff3b8552ab70ae 100644 (file)
@@ -144,10 +144,9 @@ define void @t9(<4 x float>* %r, <2 x i32>* %A) nounwind {
        store <4 x float> %tmp13, <4 x float>* %r
        ret void
 ; X64:         t9:
-; X64:                 movsd   (%rsi), %xmm0
-; X64:         movaps  (%rdi), %xmm1
-; X64:         movlhps %xmm0, %xmm1
-; X64:         movaps  %xmm1, (%rdi)
+; X64:                 movaps  (%rdi), %xmm0
+; X64:         movhps  (%rsi), %xmm0
+; X64:         movaps  %xmm0, (%rdi)
 ; X64:                 ret
 }
 
index e8628302a0c4bb0ce46e0b8a6e7b13a106f46ad0..46e59e95e53ffe5a66754afb0b63ca8e40883156 100644 (file)
@@ -1,5 +1,4 @@
-; RUN: llc < %s -march=x86 -x86-asm-syntax=intel | \
-; RUN:   grep {and     DWORD PTR} | count 2
+; RUN: llc < %s -march=x86 -x86-asm-syntax=intel | FileCheck %s
 
 target datalayout = "e-p:32:32"
         %struct.Macroblock = type { i32, i32, i32, i32, i32, [8 x i32], %struct.Macroblock*, %struct.Macroblock*, i32, [2 x [4 x [4 x [2 x i32]]]], [16 x i8], [16 x i8], i32, i64, [4 x i32], [4 x i32], i64, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i16, double, i32, i32, i32, i32, i32, i32, i32, i32, i32 }
@@ -16,5 +15,10 @@ cond_true2732.preheader:                ; preds = %entry
         %tmp2676.us.us = and i64 %tmp2667.us.us, %tmp2675not.us.us              ; <i64> [#uses=1]
         store i64 %tmp2676.us.us, i64* %tmp2666
         ret i32 0
+
+; CHECK:       and     {{E..}}, DWORD PTR [360]
+; CHECK:       and     DWORD PTR [356], {{E..}}
+; CHECK:       mov     DWORD PTR [360], {{E..}}
+
 }