[llvm-commits] [llvm] r37843 - /llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Dan Gohman djg at cray.com
Mon Jul 9 08:48:14 PDT 2007


I found a problem in an interaction between my patch and the
PPCISelLowering.cpp code for lowering call results.  The PPC target code
effectively assigns registers for a call result in reverse order. My
patch was presenting it with registers already in target order. This is
more consistent with code for other constructs, so it seems best to fix
the PPCISelLowering.cpp code.

Attached is a new patch. It's the same change to SelectionDAGISel.cpp as
before, but it's now accompanied by the PPCISelLowering.cpp update and a
test case which demonstrates the failure with the previous patch and
passes with the new one.

Thanks for your help,

Dan

On Fri, Jul 06, 2007 at 01:40:33PM -0700, Evan Cheng wrote:
> This seems to have fixed llvm-test failures, but Mac OS X PPC  
> bootstrapping is still unhappy. :-(
> 
> Evan
> 

-- 
Dan Gohman, Cray Inc.
-------------- next part --------------
Index: lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp	(revision 38457)
+++ lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp	(working copy)
@@ -621,7 +621,6 @@
                                   unsigned NumParts,
                                   MVT::ValueType PartVT,
                                   MVT::ValueType ValueVT,
-                                  bool EndianOrder,
                                   ISD::NodeType AssertOp = ISD::DELETED_NODE) {
   if (!MVT::isVector(ValueVT) || NumParts == 1) {
     SDOperand Val = Parts[0];
@@ -631,7 +630,7 @@
       assert(NumParts == 2 &&
              "Cannot expand to more than 2 elts yet!");
       SDOperand Hi = Parts[1];
-      if (EndianOrder && !DAG.getTargetLoweringInfo().isLittleEndian())
+      if (!DAG.getTargetLoweringInfo().isLittleEndian())
         std::swap(Val, Hi);
       return DAG.getNode(ISD::BUILD_PAIR, ValueVT, Val, Hi);
     }
@@ -692,7 +691,7 @@
     // as appropriate.
     for (unsigned i = 0; i != NumParts; ++i)
       Ops[i] = getCopyFromParts(DAG, &Parts[i], 1,
-                                PartVT, IntermediateVT, EndianOrder);
+                                PartVT, IntermediateVT);
   } else if (NumParts > 0) {
     // If the intermediate type was expanded, build the intermediate operands
     // from the parts.
@@ -701,7 +700,7 @@
     unsigned Factor = NumIntermediates / NumParts;
     for (unsigned i = 0; i != NumIntermediates; ++i)
       Ops[i] = getCopyFromParts(DAG, &Parts[i * Factor], Factor,
-                                PartVT, IntermediateVT, EndianOrder);
+                                PartVT, IntermediateVT);
   }
   
   // Build a vector with BUILD_VECTOR or CONCAT_VECTORS from the intermediate
@@ -718,8 +717,7 @@
                            SDOperand Val,
                            SDOperand *Parts,
                            unsigned NumParts,
-                           MVT::ValueType PartVT,
-                           bool EndianOrder) {
+                           MVT::ValueType PartVT) {
   MVT::ValueType ValueVT = Val.getValueType();
 
   if (!MVT::isVector(ValueVT) || NumParts == 1) {
@@ -728,7 +726,7 @@
       for (unsigned i = 0; i != NumParts; ++i)
         Parts[i] = DAG.getNode(ISD::EXTRACT_ELEMENT, PartVT, Val,
                                DAG.getConstant(i, MVT::i32));
-      if (EndianOrder && !DAG.getTargetLoweringInfo().isLittleEndian())
+      if (!DAG.getTargetLoweringInfo().isLittleEndian())
         std::reverse(Parts, Parts + NumParts);
       return;
     }
@@ -789,7 +787,7 @@
     // If the register was not expanded, promote or copy the value,
     // as appropriate.
     for (unsigned i = 0; i != NumParts; ++i)
-      getCopyToParts(DAG, Ops[i], &Parts[i], 1, PartVT, EndianOrder);
+      getCopyToParts(DAG, Ops[i], &Parts[i], 1, PartVT);
   } else if (NumParts > 0) {
     // If the intermediate type was expanded, split each the value into
     // legal parts.
@@ -797,7 +795,7 @@
            "Must expand into a divisible number of parts!");
     unsigned Factor = NumParts / NumIntermediates;
     for (unsigned i = 0; i != NumIntermediates; ++i)
-      getCopyToParts(DAG, Ops[i], &Parts[i * Factor], Factor, PartVT, EndianOrder);
+      getCopyToParts(DAG, Ops[i], &Parts[i * Factor], Factor, PartVT);
   }
 }
 
@@ -928,7 +926,7 @@
       unsigned NumParts = TLI.getNumRegisters(VT);
       MVT::ValueType PartVT = TLI.getRegisterType(VT);
       SmallVector<SDOperand, 4> Parts(NumParts);
-      getCopyToParts(DAG, RetOp, &Parts[0], NumParts, PartVT, true);
+      getCopyToParts(DAG, RetOp, &Parts[0], NumParts, PartVT);
       for (unsigned i = 0; i < NumParts; ++i) {
         NewValues.push_back(Parts[i]);
         NewValues.push_back(DAG.getConstant(false, MVT::i32));
@@ -2952,11 +2950,6 @@
 /// If the Flag pointer is NULL, no flag is used.
 SDOperand RegsForValue::getCopyFromRegs(SelectionDAG &DAG,
                                         SDOperand &Chain, SDOperand *Flag)const{
-  // Get the list of registers, in the appropriate order.
-  std::vector<unsigned> R(Regs);
-  if (!DAG.getTargetLoweringInfo().isLittleEndian())
-    std::reverse(R.begin(), R.end());
-
   // Copy the legal parts from the registers.
   unsigned NumParts = Regs.size();
   SmallVector<SDOperand, 8> Parts(NumParts);
@@ -2971,7 +2964,7 @@
   }
   
   // Assemble the legal parts into the final value.
-  return getCopyFromParts(DAG, &Parts[0], NumParts, RegVT, ValueVT, false);
+  return getCopyFromParts(DAG, &Parts[0], NumParts, RegVT, ValueVT);
 }
 
 /// getCopyToRegs - Emit a series of CopyToReg nodes that copies the
@@ -2980,21 +2973,16 @@
 /// If the Flag pointer is NULL, no flag is used.
 void RegsForValue::getCopyToRegs(SDOperand Val, SelectionDAG &DAG,
                                  SDOperand &Chain, SDOperand *Flag) const {
-  // Get the list of registers, in the appropriate order.
-  std::vector<unsigned> R(Regs);
-  if (!DAG.getTargetLoweringInfo().isLittleEndian())
-    std::reverse(R.begin(), R.end());
-
   // Get the list of the values's legal parts.
   unsigned NumParts = Regs.size();
   SmallVector<SDOperand, 8> Parts(NumParts);
-  getCopyToParts(DAG, Val, &Parts[0], NumParts, RegVT, false);
+  getCopyToParts(DAG, Val, &Parts[0], NumParts, RegVT);
 
   // Copy the parts into the registers.
   for (unsigned i = 0; i != NumParts; ++i) {
     SDOperand Part = Flag ?
-                     DAG.getCopyToReg(Chain, R[i], Parts[i], *Flag) :
-                     DAG.getCopyToReg(Chain, R[i], Parts[i]);
+                     DAG.getCopyToReg(Chain, Regs[i], Parts[i], *Flag) :
+                     DAG.getCopyToReg(Chain, Regs[i], Parts[i]);
     Chain = Part.getValue(0);
     if (Flag)
       *Flag = Part.getValue(1);
@@ -3867,7 +3855,7 @@
       SmallVector<SDOperand, 4> Parts(NumParts);
       for (unsigned j = 0; j != NumParts; ++j)
         Parts[j] = SDOperand(Result, i++);
-      Ops.push_back(getCopyFromParts(DAG, &Parts[0], NumParts, PartVT, VT, true));
+      Ops.push_back(getCopyFromParts(DAG, &Parts[0], NumParts, PartVT, VT));
       break;
     }
     }
@@ -3939,7 +3927,7 @@
       MVT::ValueType PartVT = getRegisterType(VT);
       unsigned NumParts = getNumRegisters(VT);
       SmallVector<SDOperand, 4> Parts(NumParts);
-      getCopyToParts(DAG, Op, &Parts[0], NumParts, PartVT, true);
+      getCopyToParts(DAG, Op, &Parts[0], NumParts, PartVT);
       for (unsigned i = 0; i != NumParts; ++i) {
         // if it isn't first piece, alignment must be 1
         unsigned MyFlags = Flags;
@@ -3979,7 +3967,7 @@
     SmallVector<SDOperand, 4> Results(NumRegs);
     for (unsigned i = 0; i != NumRegs; ++i)
       Results[i] = Res.getValue(i);
-    Res = getCopyFromParts(DAG, &Results[0], NumRegs, RegisterVT, VT, false, AssertOp);
+    Res = getCopyFromParts(DAG, &Results[0], NumRegs, RegisterVT, VT, AssertOp);
   }
 
   return std::make_pair(Res, Chain);
@@ -4269,7 +4257,7 @@
   SmallVector<SDOperand, 8> Chains(NumRegs);
 
   // Copy the value by legal parts into sequential virtual registers.
-  getCopyToParts(DAG, Op, &Regs[0], NumRegs, RegisterVT, false);
+  getCopyToParts(DAG, Op, &Regs[0], NumRegs, RegisterVT);
   for (unsigned i = 0; i != NumRegs; ++i)
     Chains[i] = DAG.getCopyToReg(getRoot(), Reg + i, Regs[i]);
   return DAG.getNode(ISD::TokenFactor, MVT::Other, &Chains[0], NumRegs);
@@ -4406,8 +4394,8 @@
   if (TI->getNumSuccessors())
     SuccsHandled.resize(BB->getParent()->getNumBlockIDs());
     
-  // Check successor nodes PHI nodes that expect a constant to be available from
-  // this block.
+  // Check successor nodes' PHI nodes that expect a constant to be available
+  // from this block.
   for (unsigned succ = 0, e = TI->getNumSuccessors(); succ != e; ++succ) {
     BasicBlock *SuccBB = TI->getSuccessor(succ);
     if (!isa<PHINode>(SuccBB->begin())) continue;
Index: lib/Target/PowerPC/PPCISelLowering.cpp
===================================================================
--- lib/Target/PowerPC/PPCISelLowering.cpp	(revision 38457)
+++ lib/Target/PowerPC/PPCISelLowering.cpp	(working copy)
@@ -1774,9 +1774,9 @@
   case MVT::Other: break;
   case MVT::i32:
     if (Op.Val->getValueType(1) == MVT::i32) {
-      Chain = DAG.getCopyFromReg(Chain, PPC::R4, MVT::i32, InFlag).getValue(1);
+      Chain = DAG.getCopyFromReg(Chain, PPC::R3, MVT::i32, InFlag).getValue(1);
       ResultVals[0] = Chain.getValue(0);
-      Chain = DAG.getCopyFromReg(Chain, PPC::R3, MVT::i32,
+      Chain = DAG.getCopyFromReg(Chain, PPC::R4, MVT::i32,
                                  Chain.getValue(2)).getValue(1);
       ResultVals[1] = Chain.getValue(0);
       NumResults = 2;
Index: test/CodeGen/PowerPC/big-endian-call-result.ll
===================================================================
--- test/CodeGen/PowerPC/big-endian-call-result.ll	(revision 0)
+++ test/CodeGen/PowerPC/big-endian-call-result.ll	(revision 0)
@@ -0,0 +1,13 @@
+; RUN: llvm-as < %s | llc -march=ppc32 -mtriple=powerpc-unknown-linux-gnu | \
+; RUN:   grep {addic 4, 4, 1}
+; RUN: llvm-as < %s | llc -march=ppc32 -mtriple=powerpc-unknown-linux-gnu | \
+; RUN:   grep {addze 3, 3}
+
+declare i64 @foo();
+
+define i64 @bar()
+{
+  %t = call i64 @foo()
+  %s = add i64 %t, 1
+  ret i64 %s
+}


More information about the llvm-commits mailing list