The new isel was not properly handling patterns that covered

internal nodes with flag results.  Record these with a new 
OPC_MarkFlagResults opcode and use this to update the interior
nodes' flag results properly.  This fixes CodeGen/X86/i256-add.ll
with the new isel.


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

include/llvm/CodeGen/DAGISelHeader.h

@@ -247,6 +247,7 @@ enum BuiltinOpcodes {
   OPC_EmitCopyToReg,
   OPC_EmitNodeXForm,
   OPC_EmitNode,
+  OPC_MarkFlagResults,
   OPC_CompleteMatch
 };
 
@@ -290,7 +291,7 @@ struct MatchScope {
   SDValue InputChain, InputFlag;
 
   /// HasChainNodesMatched - True if the ChainNodesMatched list is non-empty.
-  bool HasChainNodesMatched;
+  bool HasChainNodesMatched, HasFlagResultNodesMatched;
 };
 
 SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
@@ -354,6 +355,7 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
   // which ones they are.  The result is captured into this list so that we can
   // update the chain results when the pattern is complete.
   SmallVector<SDNode*, 3> ChainNodesMatched;
+  SmallVector<SDNode*, 3> FlagResultNodesMatched;
   
   DEBUG(errs() << "ISEL: Starting pattern match on root node: ";
         NodeToMatch->dump(CurDAG);
@@ -374,6 +376,7 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
       NewEntry.InputChain = InputChain;
       NewEntry.InputFlag = InputFlag;
       NewEntry.HasChainNodesMatched = !ChainNodesMatched.empty();
+      NewEntry.HasFlagResultNodesMatched = !FlagResultNodesMatched.empty();
       MatchScopes.push_back(NewEntry);
       continue;
     }
@@ -387,6 +390,7 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
       NewEntry.InputChain = InputChain;
       NewEntry.InputFlag = InputFlag;
       NewEntry.HasChainNodesMatched = !ChainNodesMatched.empty();
+      NewEntry.HasFlagResultNodesMatched = !FlagResultNodesMatched.empty();
       MatchScopes.push_back(NewEntry);
       continue;
     }
@@ -797,6 +801,21 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
       continue;
     }
         
+    case OPC_MarkFlagResults: {
+      unsigned NumNodes = MatcherTable[MatcherIndex++];
+      
+      // Read and remember all the flag-result nodes.
+      for (unsigned i = 0; i != NumNodes; ++i) {
+        unsigned RecNo = MatcherTable[MatcherIndex++];
+        if (RecNo & 128)
+          RecNo = GetVBR(RecNo, MatcherTable, MatcherIndex);
+
+        assert(RecNo < RecordedNodes.size() && "Invalid CheckSame");
+        FlagResultNodesMatched.push_back(RecordedNodes[RecNo].getNode());
+      }
+      continue;
+    }
+      
     case OPC_CompleteMatch: {
       // The match has been completed, and any new nodes (if any) have been
       // created.  Patch up references to the matched dag to use the newly
@@ -844,13 +863,25 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
           ReplaceUses(ChainVal, InputChain);
         }
       }
-      // If the root node produces a flag, make sure to replace its flag
+
-      // result with the resultant flag.
+      // If the result produces a flag, update any flag results in the matched
+      // pattern with the flag result.
+      if (InputFlag.getNode() != 0) {
+        // Handle the root node:
         if (NodeToMatch->getValueType(NodeToMatch->getNumValues()-1) ==
               MVT::Flag)
           ReplaceUses(SDValue(NodeToMatch, NodeToMatch->getNumValues()-1),
                       InputFlag);
         
+        // Handle any interior nodes explicitly marked.
+        for (unsigned i = 0, e = FlagResultNodesMatched.size(); i != e; ++i) {
+          SDNode *FRN = FlagResultNodesMatched[i];
+          assert(FRN->getValueType(FRN->getNumValues()-1) == MVT::Flag &&
+                 "Doesn't have a flag result");
+          ReplaceUses(SDValue(FRN, FRN->getNumValues()-1), InputFlag);
+        }
+      }
+      
       assert(NodeToMatch->use_empty() &&
              "Didn't replace all uses of the node?");
       
@@ -885,6 +916,8 @@ SDNode *SelectCodeCommon(SDNode *NodeToMatch, const unsigned char *MatcherTable,
     InputFlag = LastScope.InputFlag;
     if (!LastScope.HasChainNodesMatched)
       ChainNodesMatched.clear();
+    if (!LastScope.HasFlagResultNodesMatched)
+      FlagResultNodesMatched.clear();
 
     MatchScopes.pop_back();
   }

utils/TableGen/DAGISelMatcher.cpp

@@ -185,6 +185,11 @@ void EmitNodeMatcherNode::print(raw_ostream &OS, unsigned indent) const {
   printNext(OS, indent);
 }
 
+void MarkFlagResultsMatcherNode::print(raw_ostream &OS, unsigned indent) const {
+  OS.indent(indent) << "MarkFlagResults <todo: args>\n";
+  printNext(OS, indent);
+}
+
 void CompleteMatchMatcherNode::print(raw_ostream &OS, unsigned indent) const {
   OS.indent(indent) << "CompleteMatch <todo args>\n";
   OS.indent(indent) << "Src = " << *Pattern.getSrcPattern() << "\n";

utils/TableGen/DAGISelMatcher.h

@@ -71,6 +71,7 @@ public:
     EmitCopyToReg,        // Emit a copytoreg into a physreg.
     EmitNode,             // Create a DAG node
     EmitNodeXForm,        // Run a SDNodeXForm
+    MarkFlagResults,      // Indicate which interior nodes have flag results.
     CompleteMatch         // Finish a match and update the results.
   };
   const KindTy Kind;
@@ -623,6 +624,29 @@ public:
   virtual void print(raw_ostream &OS, unsigned indent = 0) const;
 };
   
+/// MarkFlagResultsMatcherNode - This node indicates which non-root nodes in the
+/// pattern produce flags.  This allows CompleteMatchMatcherNode to update them
+/// with the output flag of the resultant code.
+class MarkFlagResultsMatcherNode : public MatcherNode {
+  SmallVector<unsigned, 3> FlagResultNodes;
+public:
+  MarkFlagResultsMatcherNode(const unsigned *nodes, unsigned NumNodes)
+  : MatcherNode(MarkFlagResults), FlagResultNodes(nodes, nodes+NumNodes) {}
+  
+  unsigned getNumNodes() const { return FlagResultNodes.size(); }
+  
+  unsigned getNode(unsigned i) const {
+    assert(i < FlagResultNodes.size());
+    return FlagResultNodes[i];
+  }  
+  
+  static inline bool classof(const MatcherNode *N) {
+    return N->getKind() == MarkFlagResults;
+  }
+  
+  virtual void print(raw_ostream &OS, unsigned indent = 0) const;
+};
+
 /// CompleteMatchMatcherNode - Complete a match by replacing the results of the
 /// pattern with the newly generated nodes.  This also prints a comment
 /// indicating the source and dest patterns.

utils/TableGen/DAGISelMatcherEmitter.cpp

@@ -337,6 +337,15 @@ EmitMatcher(const MatcherNode *N, unsigned Indent, formatted_raw_ostream &OS) {
     OS << '\n';
     return 6+EN->getNumVTs()+NumOperandBytes;
   }
+  case MatcherNode::MarkFlagResults: {
+    const MarkFlagResultsMatcherNode *CFR = cast<MarkFlagResultsMatcherNode>(N);
+    OS << "OPC_MarkFlagResults, " << CFR->getNumNodes() << ", ";
+    unsigned NumOperandBytes = 0;
+    for (unsigned i = 0, e = CFR->getNumNodes(); i != e; ++i)
+      NumOperandBytes += EmitVBRValue(CFR->getNode(i), OS);
+    OS << '\n';
+    return 2+NumOperandBytes;
+  }
   case MatcherNode::CompleteMatch: {
     const CompleteMatchMatcherNode *CM = cast<CompleteMatchMatcherNode>(N);
     OS << "OPC_CompleteMatch, " << CM->getNumResults() << ", ";

utils/TableGen/DAGISelMatcherGen.cpp

@@ -71,6 +71,10 @@ namespace {
     /// array of all of the recorded input nodes that have chains.
     SmallVector<unsigned, 2> MatchedChainNodes;
 
+    /// MatchedFlagResultNodes - This maintains the position in the recorded
+    /// nodes array of all of the recorded input nodes that have flag results.
+    SmallVector<unsigned, 2> MatchedFlagResultNodes;
+    
     /// PhysRegInputs - List list has an entry for each explicitly specified
     /// physreg input to the pattern.  The first elt is the Register node, the
     /// second is the recorded slot number the input pattern match saved it in.
@@ -287,6 +291,9 @@ void MatcherGen::EmitLeafMatchCode(const TreePatternNode *N) {
         AddMatcherNode(new CheckChainCompatibleMatcherNode(PrevOp));
       }
     }
+    
+    // TODO: Complex patterns can't have output flags, if they did, we'd want
+    // to record them.
     return;
   }
   
@@ -417,6 +424,18 @@ void MatcherGen::EmitOperatorMatchCode(const TreePatternNode *N,
     }
   }
 
+  // If this node has an output flag and isn't the root, remember it.
+  if (N->NodeHasProperty(SDNPOutFlag, CGP) && 
+      N != Pattern.getSrcPattern()) {
+    // TODO: This redundantly records nodes with both flags and chains.
+    
+    // Record the node and remember it in our chained nodes list.
+    AddMatcherNode(new RecordMatcherNode("'" + N->getOperator()->getName() +
+                                         "' flag output node"));
+    // Remember all of the nodes with output flags our pattern will match.
+    MatchedFlagResultNodes.push_back(NextRecordedOperandNo++);
+  }
+  
   // If this node is known to have an input flag or if it *might* have an input
   // flag, capture it as the flag input of the pattern.
   if (N->NodeHasProperty(SDNPOptInFlag, CGP) ||
@@ -598,16 +617,16 @@ EmitResultInstructionAsOperand(const TreePatternNode *N,
   
   bool isRoot = N == Pattern.getDstPattern();
 
-  // NodeHasOutFlag - True if this node has a flag.
+  // TreeHasOutFlag - True if this tree has a flag.
-  bool NodeHasInFlag = false, NodeHasOutFlag = false;
+  bool TreeHasInFlag = false, TreeHasOutFlag = false;
   if (isRoot) {
     const TreePatternNode *SrcPat = Pattern.getSrcPattern();
-    NodeHasInFlag = SrcPat->TreeHasProperty(SDNPOptInFlag, CGP) ||
+    TreeHasInFlag = SrcPat->TreeHasProperty(SDNPOptInFlag, CGP) ||
                     SrcPat->TreeHasProperty(SDNPInFlag, CGP);
   
     // FIXME2: this is checking the entire pattern, not just the node in
     // question, doing this just for the root seems like a total hack.
-    NodeHasOutFlag = SrcPat->TreeHasProperty(SDNPOutFlag, CGP);
+    TreeHasOutFlag = SrcPat->TreeHasProperty(SDNPOutFlag, CGP);
   }
 
   // NumResults - This is the number of results produced by the instruction in
@@ -668,7 +687,7 @@ EmitResultInstructionAsOperand(const TreePatternNode *N,
                                                   PhysRegInputs[i].first));
     // Even if the node has no other flag inputs, the resultant node must be
     // flagged to the CopyFromReg nodes we just generated.
-    NodeHasInFlag = true;
+    TreeHasInFlag = true;
   }
   
   // Result order: node results, chain, flags
@@ -694,7 +713,7 @@ EmitResultInstructionAsOperand(const TreePatternNode *N,
   }
   if (NodeHasChain)
     ResultVTs.push_back(MVT::Other);
-  if (NodeHasOutFlag)
+  if (TreeHasOutFlag)
     ResultVTs.push_back(MVT::Flag);
 
   // FIXME2: Instead of using the isVariadic flag on the instruction, we should
@@ -727,7 +746,7 @@ EmitResultInstructionAsOperand(const TreePatternNode *N,
   AddMatcherNode(new EmitNodeMatcherNode(II.Namespace+"::"+II.TheDef->getName(),
                                          ResultVTs.data(), ResultVTs.size(),
                                          InstOps.data(), InstOps.size(),
-                                         NodeHasChain, NodeHasInFlag,
+                                         NodeHasChain, TreeHasInFlag,
                                          NodeHasMemRefs,NumFixedArityOperands));
   
   // The non-chain and non-flag results of the newly emitted node get recorded.
@@ -814,6 +833,13 @@ void MatcherGen::EmitResultCode() {
   Ops.resize(NumSrcResults);
 #endif
 
+  // If the matched pattern covers nodes which define a flag result, emit a node
+  // that tells the matcher about them so that it can update their results.
+  if (!MatchedFlagResultNodes.empty())
+    AddMatcherNode(new MarkFlagResultsMatcherNode(MatchedFlagResultNodes.data(),
+                                                MatchedFlagResultNodes.size()));
+  
+  
   // We know that the resulting pattern has exactly one result/
   // FIXME2: why?  what about something like (set a,b,c, (complexpat))
   // FIXME2: Implicit results should be pushed here I guess?