[llvm-commits] Stack and register alignment in linux/ppc calls
Chris Lattner
clattner at apple.com
Tue Mar 13 07:55:41 PDT 2007
On Mar 12, 2007, at 8:36 AM, Nicolas Geoffray wrote:
> Here's the final patch with the modifications you suggested. Thx a
> lot for your reviewing Chris.
>
> If everything's OK I'm checking this in soon.
Looks great, please do. Please also check the sabre-ppc32 tester the
day after this goes in, just to verify that there are no obvious
darwin regressions. Thanks!
-Chris
> Cheers,
> Nicolas
>
> Chris Lattner wrote:
>>
>> On Mar 6, 2007, at 10:03 AM, Nicolas Geoffray wrote:
>>> This patch corrects arguments passing alignment for linux/ppc
>>> calls (ELF ABI).
>>> It affects LowerFORMAL_ARGUMENTS and LowerCALL of
>>> PPCISelLowering.cpp.
>>
>> Sure, sorry for the delay. Please add some high-level comments
>> that explain what is going on here (what the ABI says). I would
>> eventually like to switch PPC over to using autogenerated
>> callingconv code, but I haven't had a chance to finish argument
>> passing.
>>
>>> @@ -1164,24 +1165,34 @@ static SDOperand LowerFORMAL_ARGUMENTS(S
>>> SDOperand ArgVal;
>>> bool needsLoad = false;
>>> MVT::ValueType ObjectVT = Op.getValue(ArgNo).getValueType();
>>> unsigned ObjSize = MVT::getSizeInBits(ObjectVT)/8;
>>> unsigned ArgSize = ObjSize;
>>> + unsigned Flags = cast<ConstantSDNode>(Op.getOperand(ArgNo
>>> +3))->getValue();
>>> + // See if next argument requires stack alignment in ELF
>>> + unsigned Expand = (ObjectVT == MVT::f64) || ((ArgNo + 1 < e) &&
>>> + (cast<ConstantSDNode>(Op.getOperand(ArgNo+4))->getValue()
>>> & (1 << 27)) &&
>>> + (!(Flags & (1 << 27))));
>>
>> Please update this to use the enums that anton recently added for
>> decoding the flags values.
>>
>>> unsigned CurArgOffset = ArgOffset;
>>> switch (ObjectVT) {
>>> default: assert(0 && "Unhandled argument type!");
>>> case MVT::i32:
>>> + // Double word align in ELF
>>> + if (Expand && !isELF_ABI && !isPPC64) GPR_idx += (GPR_idx
>>> % 2);
>>
>> This says "!isELF_ABI", shouldn't it be "isELF_ABI"? If not,
>> you're modifying the Darwin/PPC ABI.
>>
>>> -
>>> + unsigned Flags = cast<ConstantSDNode>(Op.getOperand(5+2*i
>>> +1))->getValue();
>>> + // See if next argument requires stack alignment in ELF
>>> + unsigned Expand = (Arg.getValueType() == MVT::f64) ||
>>> + ((i + 1 < NumOps) &&
>>> + (cast<ConstantSDNode>(Op.getOperand(5+2*(i+1)+1))->getValue()
>>> + & (1
>>> << 27)) &&
>>> + (!(Flags & (1 << 27))));
>>
>> Likewise, plz use enums here. Also, there is some funky
>> indentation going on here. Perhaps making a "ConstantSDNode *Tmp"
>> would make this more natural.
>>
>>> // PtrOff will be used to store the current argument to the
>>> stack if a
>>> // register cannot be found for it.
>>> - SDOperand PtrOff = DAG.getConstant(ArgOffset,
>>> StackPtr.getValueType());
>>> + SDOperand PtrOff;
>>> +
>>> + // Stack align in ELF
>>> + if (isELF_ABI && Expand && !isPPC64)
>>> + PtrOff = DAG.getConstant(ArgOffset + ((ArgOffset/4) % 2)
>>> * PtrByteSize,
>>> + StackPtr.getValueType());
>>> + else
>>> + PtrOff = DAG.getConstant(ArgOffset, StackPtr.getValueType
>>> ());
>>> +
>>
>> Funky indentation. Statements should be indented by 2.
>> Subexpressions (StackPtr.getValueType() should be aligned to the (.
>>
>> Otherwise, looks great, thanks!
>>
>> -Chris
>
> Index: PPCISelLowering.cpp
> ===================================================================
> RCS file: /var/cvs/llvm/llvm/lib/Target/PowerPC/PPCISelLowering.cpp,v
> retrieving revision 1.260
> diff -t -d -u -p -5 -r1.260 PPCISelLowering.cpp
> --- PPCISelLowering.cpp 6 Mar 2007 00:59:59 -0000 1.260
> +++ PPCISelLowering.cpp 12 Mar 2007 15:39:25 -0000
> @@ -1130,10 +1130,11 @@ static SDOperand LowerFORMAL_ARGUMENTS(S
> SDOperand Root = Op.getOperand(0);
>
> MVT::ValueType PtrVT = DAG.getTargetLoweringInfo().getPointerTy();
> bool isPPC64 = PtrVT == MVT::i64;
> bool isMachoABI = Subtarget.isMachoABI();
> + bool isELF_ABI = Subtarget.isELF_ABI();
> unsigned PtrByteSize = isPPC64 ? 8 : 4;
>
> unsigned ArgOffset = PPCFrameInfo::getLinkageSize(isPPC64,
> isMachoABI);
>
> static const unsigned GPR_32[] = { // 32-bit registers.
> @@ -1161,30 +1162,46 @@ static SDOperand LowerFORMAL_ARGUMENTS(S
> const unsigned *GPR = isPPC64 ? GPR_64 : GPR_32;
>
> // Add DAG nodes to load the arguments or copy them out of
> registers. On
> // entry to a function on PPC, the arguments start after the
> linkage area,
> // although the first ones are often in registers.
> + //
> + // In the ELF ABI, GPRs and stack are double word align: an
> argument
> + // represented with two words (long long or double) must be
> copied to an
> + // even GPR_idx value or to an even ArgOffset value.
> +
> for (unsigned ArgNo = 0, e = Op.Val->getNumValues()-1; ArgNo !=
> e; ++ArgNo) {
> SDOperand ArgVal;
> bool needsLoad = false;
> MVT::ValueType ObjectVT = Op.getValue(ArgNo).getValueType();
> unsigned ObjSize = MVT::getSizeInBits(ObjectVT)/8;
> unsigned ArgSize = ObjSize;
> + unsigned Flags = cast<ConstantSDNode>(Op.getOperand(ArgNo+3))-
> >getValue();
> + unsigned AlignFlag = 1 << ISD::ParamFlags::OrigAlignmentOffs;
> + // See if next argument requires stack alignment in ELF
> + bool Expand = (ObjectVT == MVT::f64) || ((ArgNo + 1 < e) &&
> + (cast<ConstantSDNode>(Op.getOperand(ArgNo+4))->getValue() &
> AlignFlag) &&
> + (!(Flags & AlignFlag)));
>
> unsigned CurArgOffset = ArgOffset;
> switch (ObjectVT) {
> default: assert(0 && "Unhandled argument type!");
> case MVT::i32:
> + // Double word align in ELF
> + if (Expand && isELF_ABI && !isPPC64) GPR_idx += (GPR_idx % 2);
> if (GPR_idx != Num_GPR_Regs) {
> unsigned VReg = RegMap->createVirtualRegister
> (&PPC::GPRCRegClass);
> MF.addLiveIn(GPR[GPR_idx], VReg);
> ArgVal = DAG.getCopyFromReg(Root, VReg, MVT::i32);
> ++GPR_idx;
> } else {
> needsLoad = true;
> ArgSize = PtrByteSize;
> }
> + // Stack align in ELF
> + if (needsLoad && Expand && isELF_ABI && !isPPC64)
> + ArgOffset += ((ArgOffset/4) % 2) * PtrByteSize;
> // All int arguments reserve stack space in Macho ABI.
> if (isMachoABI || needsLoad) ArgOffset += PtrByteSize;
> break;
>
> case MVT::i64: // PPC64
> @@ -1202,11 +1219,11 @@ static SDOperand LowerFORMAL_ARGUMENTS(S
>
> case MVT::f32:
> case MVT::f64:
> // Every 4 bytes of argument space consumes one of the GPRs
> available for
> // argument passing.
> - if (GPR_idx != Num_GPR_Regs) {
> + if (GPR_idx != Num_GPR_Regs && isMachoABI) {
> ++GPR_idx;
> if (ObjSize == 8 && GPR_idx != Num_GPR_Regs && !isPPC64)
> ++GPR_idx;
> }
> if (FPR_idx != Num_FPR_Regs) {
> @@ -1220,10 +1237,13 @@ static SDOperand LowerFORMAL_ARGUMENTS(S
> ++FPR_idx;
> } else {
> needsLoad = true;
> }
>
> + // Stack align in ELF
> + if (needsLoad && Expand && isELF_ABI && !isPPC64)
> + ArgOffset += ((ArgOffset/4) % 2) * PtrByteSize;
> // All FP arguments reserve stack space in Macho ABI.
> if (isMachoABI || needsLoad) ArgOffset += isPPC64 ? 8 :
> ObjSize;
> break;
> case MVT::v4f32:
> case MVT::v4i32:
> @@ -1322,10 +1342,11 @@ static SDOperand LowerCALL(SDOperand Op,
> bool isVarArg = cast<ConstantSDNode>(Op.getOperand(2))-
> >getValue() != 0;
> SDOperand Callee = Op.getOperand(4);
> unsigned NumOps = (Op.getNumOperands() - 5) / 2;
>
> bool isMachoABI = Subtarget.isMachoABI();
> + bool isELF_ABI = Subtarget.isELF_ABI();
>
> MVT::ValueType PtrVT = DAG.getTargetLoweringInfo().getPointerTy();
> bool isPPC64 = PtrVT == MVT::i64;
> unsigned PtrByteSize = isPPC64 ? 8 : 4;
>
> @@ -1397,35 +1418,57 @@ static SDOperand LowerCALL(SDOperand Op,
> std::vector<std::pair<unsigned, SDOperand> > RegsToPass;
> SmallVector<SDOperand, 8> MemOpChains;
> for (unsigned i = 0; i != NumOps; ++i) {
> bool inMem = false;
> SDOperand Arg = Op.getOperand(5+2*i);
> -
> + unsigned Flags = cast<ConstantSDNode>(Op.getOperand(5+2*i+1))-
> >getValue();
> + unsigned AlignFlag = 1 << ISD::ParamFlags::OrigAlignmentOffs;
> + // See if next argument requires stack alignment in ELF
> + unsigned next = 5+2*(i+1)+1;
> + bool Expand = (Arg.getValueType() == MVT::f64) || ((i + 1 <
> NumOps) &&
> + (cast<ConstantSDNode>(Op.getOperand(next))->getValue() &
> AlignFlag) &&
> + (!(Flags & AlignFlag)));
> +
> // PtrOff will be used to store the current argument to the
> stack if a
> // register cannot be found for it.
> - SDOperand PtrOff = DAG.getConstant(ArgOffset,
> StackPtr.getValueType());
> + SDOperand PtrOff;
> +
> + // Stack align in ELF
> + if (isELF_ABI && Expand && !isPPC64)
> + PtrOff = DAG.getConstant(ArgOffset + ((ArgOffset/4) % 2) *
> PtrByteSize,
> + StackPtr.getValueType());
> + else
> + PtrOff = DAG.getConstant(ArgOffset, StackPtr.getValueType());
> +
> PtrOff = DAG.getNode(ISD::ADD, PtrVT, StackPtr, PtrOff);
>
> // On PPC64, promote integers to 64-bit values.
> if (isPPC64 && Arg.getValueType() == MVT::i32) {
> - unsigned Flags = cast<ConstantSDNode>(Op.getOperand(5+2*i
> +1))->getValue();
> unsigned ExtOp = (Flags & 1) ? ISD::SIGN_EXTEND :
> ISD::ZERO_EXTEND;
>
> Arg = DAG.getNode(ExtOp, MVT::i64, Arg);
> }
>
> switch (Arg.getValueType()) {
> default: assert(0 && "Unexpected ValueType for argument!");
> case MVT::i32:
> case MVT::i64:
> + // Double word align in ELF
> + if (isELF_ABI && Expand && !isPPC64) GPR_idx += (GPR_idx % 2);
> if (GPR_idx != NumGPRs) {
> RegsToPass.push_back(std::make_pair(GPR[GPR_idx++], Arg));
> } else {
> MemOpChains.push_back(DAG.getStore(Chain, Arg, PtrOff,
> NULL, 0));
> inMem = true;
> }
> - if (inMem || isMachoABI) ArgOffset += PtrByteSize;
> + if (inMem || isMachoABI) {
> + // Stack align in ELF
> + if (isELF_ABI && Expand && !isPPC64)
> + ArgOffset += ((ArgOffset/4) % 2) * PtrByteSize;
> +
> + ArgOffset += PtrByteSize;
> + }
> break;
> case MVT::f32:
> case MVT::f64:
> if (isVarArg) {
> // Float varargs need to be promoted to double.
> @@ -1470,10 +1513,13 @@ static SDOperand LowerCALL(SDOperand Op,
> } else {
> MemOpChains.push_back(DAG.getStore(Chain, Arg, PtrOff,
> NULL, 0));
> inMem = true;
> }
> if (inMem || isMachoABI) {
> + // Stack align in ELF
> + if (isELF_ABI && Expand && !isPPC64)
> + ArgOffset += ((ArgOffset/4) % 2) * PtrByteSize;
> if (isPPC64)
> ArgOffset += 8;
> else
> ArgOffset += Arg.getValueType() == MVT::f32 ? 4 : 8;
> }
> @@ -1501,11 +1547,11 @@ static SDOperand LowerCALL(SDOperand Op,
> InFlag);
> InFlag = Chain.getValue(1);
> }
>
> // With the ELF ABI, set CR6 to true if this is a vararg call.
> - if (isVarArg && !isMachoABI) {
> + if (isVarArg && isELF_ABI) {
> SDOperand SetCR(DAG.getTargetNode(PPC::SETCR, MVT::i32), 0);
> Chain = DAG.getCopyToReg(Chain, PPC::CR6, SetCR, InFlag);
> InFlag = Chain.getValue(1);
> }
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list