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@cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> >> <patch> _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
