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

Evan Cheng evan.cheng at apple.com
Thu Jul 5 14:07:02 PDT 2007


On Jul 5, 2007, at 7:49 AM, Dan Gohman wrote:

> Hi Evan,
>
> I'm currently testing the attached patch, which seems to fix the
> problems for PPC. It extends getCopyToParts and getCopyFromParts
> to have an extra parameter to indicate when endian-swapping is
> needed. I don't have access to a PPC test environment; I've included
> a few trivial regression tests in this patch; I'll do some more
> soon.

Thanks. I'll do some testing.

>
> Copying to virtual registers is always done in little-endian order,
> while copying to physical registers, arguments, or return values,
> is done in target-endian order. I wonder if it would make sense to
> change to using target-endian order for virtual registers as well?

I don't think it matters, does it? If it makes the code cleaner then  
please do so.

Thanks,

Evan

>
> Dan
>
> On Tue, Jul 03, 2007 at 06:35:58PM -0700, Evan Cheng wrote:
>> Hi Dan,
>>
>> This patch is breaking llvm-gcc bootstrapping on PPC.
>>
>> I am not sure what exactly wrong is it. But the old code has a check
>> for endianness while your new code doesn't. Can you check again if
>> you are taking endianness into consideration?
>>
>> Thanks,
>>
>> Evan
>>
>> On Jul 2, 2007, at 9:18 AM, Dan Gohman wrote:
>>
>>> Author: djg
>>> Date: Mon Jul  2 11:18:06 2007
>>> New Revision: 37843
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=37843&view=rev
>>> Log:
>>> Replace ExpandScalarFormalArgs and ExpandScalarCallArgs with the  
>>> newly
>>> refactored getCopyFromParts and getCopyToParts, which are more
>>> general.
>>> This effectively adds support for lowering illegal by-val vector  
>>> call
>>> arguments.
>>>
>>> Modified:
>>>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/
>>> SelectionDAG/SelectionDAGISel.cpp?
>>> rev=37843&r1=37842&r2=37843&view=diff
>>> ==================================================================== 
>>> ==
>>> ========
>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>>> (original)
>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon
>>> Jul  2 11:18:06 2007
>>> @@ -2861,7 +2861,7 @@
>>>   if (!MVT::isVector(ValueVT) || NumParts == 1) {
>>>     // If the value was expanded, copy from the parts.
>>>     if (NumParts > 1) {
>>> -      for (unsigned i = 0; i < NumParts; ++i)
>>> +      for (unsigned i = 0; i != NumParts; ++i)
>>>         Parts[i] = DAG.getNode(ISD::EXTRACT_ELEMENT, PartVT, Val,
>>>                                DAG.getConstant(i, MVT::i32));
>>>       return;
>>> @@ -2950,7 +2950,7 @@
>>>   // Copy the legal parts from the registers.
>>>   unsigned NumParts = Regs.size();
>>>   SmallVector<SDOperand, 8> Parts(NumParts);
>>> -  for (unsigned i = 0; i < NumParts; ++i) {
>>> +  for (unsigned i = 0; i != NumParts; ++i) {
>>>     SDOperand Part = Flag ?
>>>                      DAG.getCopyFromReg(Chain, Regs[i], RegVT,
>>> *Flag) :
>>>                      DAG.getCopyFromReg(Chain, Regs[i], RegVT);
>>> @@ -2981,7 +2981,7 @@
>>>   getCopyToParts(DAG, Val, &Parts[0], NumParts, RegVT);
>>>
>>>   // Copy the parts into the registers.
>>> -  for (unsigned i = 0; i < NumParts; ++i) {
>>> +  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]);
>>> @@ -3746,32 +3746,6 @@
>>>                           DAG.getSrcValue(I.getOperand(2))));
>>> }
>>>
>>> -/// ExpandScalarFormalArgs - Recursively expand the
>>> formal_argument node, either
>>> -/// bit_convert it or join a pair of them with a BUILD_PAIR when
>>> appropriate.
>>> -static SDOperand ExpandScalarFormalArgs(MVT::ValueType VT, SDNode
>>> *Arg,
>>> -                                        unsigned &i, SelectionDAG
>>> &DAG,
>>> -                                        TargetLowering &TLI) {
>>> -  if (TLI.getTypeAction(VT) != TargetLowering::Expand)
>>> -    return SDOperand(Arg, i++);
>>> -
>>> -  MVT::ValueType EVT = TLI.getTypeToTransformTo(VT);
>>> -  unsigned NumVals = MVT::getSizeInBits(VT) / MVT::getSizeInBits
>>> (EVT);
>>> -  if (NumVals == 1) {
>>> -    return DAG.getNode(ISD::BIT_CONVERT, VT,
>>> -                       ExpandScalarFormalArgs(EVT, Arg, i, DAG,
>>> TLI));
>>> -  } else if (NumVals == 2) {
>>> -    SDOperand Lo = ExpandScalarFormalArgs(EVT, Arg, i, DAG, TLI);
>>> -    SDOperand Hi = ExpandScalarFormalArgs(EVT, Arg, i, DAG, TLI);
>>> -    if (!TLI.isLittleEndian())
>>> -      std::swap(Lo, Hi);
>>> -    return DAG.getNode(ISD::BUILD_PAIR, VT, Lo, Hi);
>>> -  } else {
>>> -    // Value scalarized into many values.  Unimp for now.
>>> -    assert(0 && "Cannot expand i64 -> i16 yet!");
>>> -  }
>>> -  return SDOperand();
>>> -}
>>> -
>>> /// TargetLowering::LowerArguments - This is the default
>>> LowerArguments
>>> /// implementation, which just inserts a FORMAL_ARGUMENTS node.
>>> FIXME: When all
>>> /// targets are migrated to using FORMAL_ARGUMENTS, this hook
>>> should be
>>> @@ -3842,8 +3816,8 @@
>>>   SDNode *Result = DAG.getNode(ISD::FORMAL_ARGUMENTS,
>>>                                DAG.getNodeValueTypes(RetVals),
>>> RetVals.size(),
>>>                                &Ops[0], Ops.size()).Val;
>>> -
>>> -  DAG.setRoot(SDOperand(Result, Result->getNumValues()-1));
>>> +  unsigned NumArgRegs = Result->getNumValues() - 1;
>>> +  DAG.setRoot(SDOperand(Result, NumArgRegs));
>>>
>>>   // Set up the return result vector.
>>>   Ops.clear();
>>> @@ -3875,79 +3849,22 @@
>>>       Ops.push_back(Op);
>>>       break;
>>>     }
>>> -    case Expand:
>>> -      if (!MVT::isVector(VT)) {
>>> -        // If this is a large integer or a floating point node
>>> that needs to be
>>> -        // expanded, it needs to be reassembled from small
>>> integers.  Figure out
>>> -        // what the source elt type is and how many small integers
>>> it is.
>>> -        Ops.push_back(ExpandScalarFormalArgs(VT, Result, i, DAG,
>>> *this));
>>> -      } else {
>>> -        // Otherwise, this is a vector type.  We only support
>>> legal vectors
>>> -        // right now.
>>> -        const VectorType *PTy = cast<VectorType>(I->getType());
>>> -        unsigned NumElems = PTy->getNumElements();
>>> -        const Type *EltTy = PTy->getElementType();
>>> -
>>> -        // Figure out if there is a Packed type corresponding to
>>> this Vector
>>> -        // type.  If so, convert to the vector type.
>>> -        MVT::ValueType TVT =
>>> -          MVT::getVectorType(getValueType(EltTy), NumElems);
>>> -        if (TVT != MVT::Other && isTypeLegal(TVT)) {
>>> -          SDOperand N = SDOperand(Result, i++);
>>> -          // Handle copies from vectors to registers.
>>> -          N = DAG.getNode(ISD::BIT_CONVERT, TVT, N);
>>> -          Ops.push_back(N);
>>> -        } else {
>>> -          assert(0 && "Don't support illegal by-val vector
>>> arguments yet!");
>>> -          abort();
>>> -        }
>>> -      }
>>> +    case Expand: {
>>> +      MVT::ValueType PartVT = getRegisterType(VT);
>>> +      unsigned NumParts = getNumRegisters(VT);
>>> +      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));
>>>       break;
>>>     }
>>> +    }
>>>   }
>>> +  assert(i == NumArgRegs && "Argument register count mismatch!");
>>>   return Ops;
>>> }
>>>
>>>
>>> -/// ExpandScalarCallArgs - Recursively expand call argument node by
>>> -/// bit_converting it or extract a pair of elements from the
>>> larger  node.
>>> -static void ExpandScalarCallArgs(MVT::ValueType VT, SDOperand Arg,
>>> -                                 unsigned Flags,
>>> -                                 SmallVector<SDOperand, 32> &Ops,
>>> -                                 SelectionDAG &DAG,
>>> -                                 TargetLowering &TLI,
>>> -                                 bool isFirst = true) {
>>> -
>>> -  if (TLI.getTypeAction(VT) != TargetLowering::Expand) {
>>> -    // if it isn't first piece, alignment must be 1
>>> -    if (!isFirst)
>>> -      Flags = (Flags & (~ISD::ParamFlags::OrigAlignment)) |
>>> -        (1 << ISD::ParamFlags::OrigAlignmentOffs);
>>> -    Ops.push_back(Arg);
>>> -    Ops.push_back(DAG.getConstant(Flags, MVT::i32));
>>> -    return;
>>> -  }
>>> -
>>> -  MVT::ValueType EVT = TLI.getTypeToTransformTo(VT);
>>> -  unsigned NumVals = MVT::getSizeInBits(VT) / MVT::getSizeInBits
>>> (EVT);
>>> -  if (NumVals == 1) {
>>> -    Arg = DAG.getNode(ISD::BIT_CONVERT, EVT, Arg);
>>> -    ExpandScalarCallArgs(EVT, Arg, Flags, Ops, DAG, TLI, isFirst);
>>> -  } else if (NumVals == 2) {
>>> -    SDOperand Lo = DAG.getNode(ISD::EXTRACT_ELEMENT, EVT, Arg,
>>> -                               DAG.getConstant(0, TLI.getPointerTy
>>> ()));
>>> -    SDOperand Hi = DAG.getNode(ISD::EXTRACT_ELEMENT, EVT, Arg,
>>> -                               DAG.getConstant(1, TLI.getPointerTy
>>> ()));
>>> -    if (!TLI.isLittleEndian())
>>> -      std::swap(Lo, Hi);
>>> -    ExpandScalarCallArgs(EVT, Lo, Flags, Ops, DAG, TLI, isFirst);
>>> -    ExpandScalarCallArgs(EVT, Hi, Flags, Ops, DAG, TLI, false);
>>> -  } else {
>>> -    // Value scalarized into many values.  Unimp for now.
>>> -    assert(0 && "Cannot expand i64 -> i16 yet!");
>>> -  }
>>> -}
>>> -
>>> /// TargetLowering::LowerCallTo - This is the default LowerCallTo
>>> /// implementation, which just inserts an ISD::CALL node, which is
>>> later custom
>>> /// lowered by the target to something concrete.  FIXME: When all
>>> targets are
>>> @@ -4014,35 +3931,24 @@
>>>       Ops.push_back(Op);
>>>       Ops.push_back(DAG.getConstant(Flags, MVT::i32));
>>>       break;
>>> -    case Expand:
>>> -      if (!MVT::isVector(VT)) {
>>> -        // If this is a large integer, it needs to be broken down
>>> into small
>>> -        // integers.  Figure out what the source elt type is and
>>> how many small
>>> -        // integers it is.
>>> -        ExpandScalarCallArgs(VT, Op, Flags, Ops, DAG, *this);
>>> -      } else {
>>> -        // Otherwise, this is a vector type.  We only support
>>> legal vectors
>>> -        // right now.
>>> -        const VectorType *PTy = cast<VectorType>(Args[i].Ty);
>>> -        unsigned NumElems = PTy->getNumElements();
>>> -        const Type *EltTy = PTy->getElementType();
>>> -
>>> -        // Figure out if there is a Packed type corresponding to
>>> this Vector
>>> -        // type.  If so, convert to the vector type.
>>> -        MVT::ValueType TVT =
>>> -          MVT::getVectorType(getValueType(EltTy), NumElems);
>>> -        if (TVT != MVT::Other && isTypeLegal(TVT)) {
>>> -          // Insert a BIT_CONVERT of the original type to the
>>> vector type.
>>> -          Op = DAG.getNode(ISD::BIT_CONVERT, TVT, Op);
>>> -          Ops.push_back(Op);
>>> -          Ops.push_back(DAG.getConstant(Flags, MVT::i32));
>>> -        } else {
>>> -          assert(0 && "Don't support illegal by-val vector call
>>> args yet!");
>>> -          abort();
>>> -        }
>>> +    case Expand: {
>>> +      MVT::ValueType PartVT = getRegisterType(VT);
>>> +      unsigned NumParts = getNumRegisters(VT);
>>> +      SmallVector<SDOperand, 4> Parts(NumParts);
>>> +      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;
>>> +        if (i != 0)
>>> +          MyFlags = (MyFlags & (~ISD::ParamFlags::OrigAlignment)) |
>>> +            (1 << ISD::ParamFlags::OrigAlignmentOffs);
>>> +
>>> +        Ops.push_back(Parts[i]);
>>> +        Ops.push_back(DAG.getConstant(MyFlags, MVT::i32));
>>>       }
>>>       break;
>>>     }
>>> +    }
>>>   }
>>>
>>>   // Figure out the result value types.
>>> @@ -4360,7 +4266,7 @@
>>>
>>>   // Copy the value by legal parts into sequential virtual  
>>> registers.
>>>   getCopyToParts(DAG, Op, &Regs[0], NumRegs, RegisterVT);
>>> -  for (unsigned i = 0; i < NumRegs; ++i)
>>> +  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);
>>> }
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>> <patch>




More information about the llvm-commits mailing list