[llvm-commits] [llvm] r96767 - in /llvm/trunk: include/llvm/CodeGen/DAGISelHeader.h utils/TableGen/DAGISelMatcherGen.cpp

Chris Lattner sabre at nondot.org
Sun Feb 21 15:54:05 PST 2010


Author: lattner
Date: Sun Feb 21 17:54:05 2010
New Revision: 96767

URL: http://llvm.org/viewvc/llvm-project?rev=96767&view=rev
Log:
fix most of the failures in the x86 suite by handling multiple 
result nodes correctly.  Note that this includes a horrible hack
in DAGISelHeader which cannot be fixed reasonably without 
eliminating (parallel) from input patterns.  That, in turn,
can't be done until we support writing multiple result patterns
for the X86and_flag and related multiple-result nodes.

Modified:
    llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h
    llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp

Modified: llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h?rev=96767&r1=96766&r2=96767&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h (original)
+++ llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h Sun Feb 21 17:54:05 2010
@@ -695,7 +695,11 @@
                                                   NodeToMatch->getDebugLoc(),
                                                   VTList,
                                                   Ops.data(), Ops.size());
-      RecordedNodes.push_back(SDValue(Res, 0));
+      // Add all the non-flag/non-chain results to the RecordedNodes list.
+      for (unsigned i = 0, e = VTs.size(); i != e; ++i) {
+        if (VTs[i] == MVT::Other || VTs[i] == MVT::Flag) break;
+        RecordedNodes.push_back(SDValue(Res, i));
+      }
       
       // If the node had chain/flag results, update our notion of the current
       // chain and flag.
@@ -731,6 +735,13 @@
         unsigned ResSlot = MatcherTable[MatcherIndex++];
         assert(ResSlot < RecordedNodes.size() && "Invalid CheckSame");
         SDValue Res = RecordedNodes[ResSlot];
+        
+        // FIXME2: Eliminate this horrible hack by fixing the 'Gen' program
+        // after (parallel) on input patterns are removed.  This would also
+        // allow us to stop encoding #results in OPC_CompleteMatch's table
+        // entry.
+        if (NodeToMatch->getNumValues() <= i)
+          break;
         assert((NodeToMatch->getValueType(i) == Res.getValueType() ||
                 NodeToMatch->getValueType(i) == MVT::iPTR ||
                 Res.getValueType() == MVT::iPTR ||
@@ -763,6 +774,8 @@
         ReplaceUses(SDValue(NodeToMatch, NodeToMatch->getNumValues()-1),
                     InputFlag);
       
+      assert(NodeToMatch->use_empty() &&
+             "Didn't replace all uses of the node?");
       // FIXME: We just return here, which interacts correctly with SelectRoot
       // above.  We should fix this to not return an SDNode* anymore.
       return 0;

Modified: llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp?rev=96767&r1=96766&r2=96767&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp (original)
+++ llvm/trunk/utils/TableGen/DAGISelMatcherGen.cpp Sun Feb 21 17:54:05 2010
@@ -666,7 +666,7 @@
   
   // Determine the result types.
   SmallVector<MVT::SimpleValueType, 4> ResultVTs;
-  if (NumResults > 0 && N->getTypeNum(0) != MVT::isVoid) {
+  if (NumResults != 0 && N->getTypeNum(0) != MVT::isVoid) {
     // FIXME2: If the node has multiple results, we should add them.  For now,
     // preserve existing behavior?!
     ResultVTs.push_back(N->getTypeNum(0));
@@ -721,10 +721,11 @@
                                          NodeHasChain, NodeHasInFlag,
                                          NodeHasMemRefs,NumFixedArityOperands));
   
-  // The newly emitted node gets recorded.
-  // FIXME2: This should record all of the results except the (implicit) one.
-  if (ResultVTs[0] != MVT::Other)
+  // The non-chain and non-flag results of the newly emitted node get recorded.
+  for (unsigned i = 0, e = ResultVTs.size(); i != e; ++i) {
+    if (ResultVTs[i] == MVT::Other || ResultVTs[i] == MVT::Flag) break;
     OutputOps.push_back(NextRecordedOperandNo++);
+  }
   
   // FIXME2: Kill off all the SelectionDAG::SelectNodeTo and getMachineNode
   // variants.  Call MorphNodeTo instead of SelectNodeTo.
@@ -770,16 +771,43 @@
 }
 
 void MatcherGen::EmitResultCode() {
+  // Codegen the root of the result pattern, capturing the resulting values.
   SmallVector<unsigned, 8> Ops;
   EmitResultOperand(Pattern.getDstPattern(), Ops);
 
+  // At this point, we have however many values the result pattern produces.
+  // However, the input pattern might not need all of these.  If there are
+  // excess values at the end (such as condition codes etc) just lop them off.
+  // This doesn't need to worry about flags or chains, just explicit results.
+  //
+  // FIXME2: This doesn't work because there is currently no way to get an
+  // accurate count of the # results the source pattern sets.  This is because
+  // of the "parallel" construct in X86 land, which looks like this:
+  //
+  //def : Pat<(parallel (X86and_flag GR8:$src1, GR8:$src2),
+  //           (implicit EFLAGS)),
+  //  (AND8rr GR8:$src1, GR8:$src2)>;
+  //
+  // This idiom means to match the two-result node X86and_flag (which is
+  // declared as returning a single result, because we can't match multi-result
+  // nodes yet).  In this case, we would have to know that the input has two
+  // results.  However, mul8r is modelled exactly the same way, but without
+  // implicit defs included.  The fix is to support multiple results directly
+  // and eliminate 'parallel'.
+  //
+  // FIXME2: When this is fixed, we should revert the terrible hack in the
+  // OPC_EmitNode code in the interpreter.
+#if 0
+  const TreePatternNode *Src = Pattern.getSrcPattern();
+  unsigned NumSrcResults = Src->getTypeNum(0) != MVT::isVoid ? 1 : 0;
+  NumSrcResults += Pattern.getDstRegs().size();
+  assert(Ops.size() >= NumSrcResults && "Didn't provide enough results");
+  Ops.resize(NumSrcResults);
+#endif
+  
   // 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?
-  assert(Ops.size() <= 1);
-  // FIXME: Handle Ops.
-  // FIXME: Handle (set EAX, (foo)) but not (implicit EFLAGS)
-  
   AddMatcherNode(new CompleteMatchMatcherNode(Ops.data(), Ops.size(), Pattern));
 }
 





More information about the llvm-commits mailing list