Diff 377615

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

This file is larger than 256 KB, so syntax highlighting is disabled by default.

Show First 20 Lines • Show All 4,211 Lines • ▼ Show 20 Lines ArgVal = DAG.getNode(ISD::AssertSext, dl, MVT::i64, ArgVal,

DAG.getValueType(ObjectVT)); DAG.getValueType(ObjectVT));

else if (Flags.isZExt()) else if (Flags.isZExt())

ArgVal = DAG.getNode(ISD::AssertZext, dl, MVT::i64, ArgVal, ArgVal = DAG.getNode(ISD::AssertZext, dl, MVT::i64, ArgVal,

DAG.getValueType(ObjectVT)); DAG.getValueType(ObjectVT));

return DAG.getNode(ISD::TRUNCATE, dl, ObjectVT, ArgVal); return DAG.getNode(ISD::TRUNCATE, dl, ObjectVT, ArgVal);

} }

SDValue PPCTargetLowering::LowerFormalArguments_64SVR4( SDValue PPCTargetLowering::LowerFormalArguments_64SVR4(

amykUnsubmitted

Not Done

Can we add documentation to this function?

amyk: Can we add documentation to this function?

SDValue Chain, CallingConv::ID CallConv, bool isVarArg, SDValue Chain, CallingConv::ID CallConv, bool isVarArg,

const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl, const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,

SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const { SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {

amykUnsubmitted

Not Done

// |-------- 2 bytes --------|---- 1 byte ----|

- // and we want to store the 1 byte element we would call this funciton

+ // and we want to store the 1 byte element we would call this function

// with the Offset = 2 (as we want to jump over the first 2 bytes.)

amyk:

// TODO: add description of PPC stack frame format, or at least some docs. // TODO: add description of PPC stack frame format, or at least some docs.

// //

bool isELFv2ABI = Subtarget.isELFv2ABI(); bool isELFv2ABI = Subtarget.isELFv2ABI();

bool isLittleEndian = Subtarget.isLittleEndian(); bool isLittleEndian = Subtarget.isLittleEndian();

MachineFunction &MF = DAG.getMachineFunction(); MachineFunction &MF = DAG.getMachineFunction();

MachineFrameInfo &MFI = MF.getFrameInfo(); MachineFrameInfo &MFI = MF.getFrameInfo();

PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>(); PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();

assert(!(CallConv == CallingConv::Fast && isVarArg) && assert(!(CallConv == CallingConv::Fast && isVarArg) &&

"fastcc not supported on varargs functions"); "fastcc not supported on varargs functions");

EVT PtrVT = getPointerTy(MF.getDataLayout()); EVT PtrVT = getPointerTy(MF.getDataLayout());

// Potential tail calls could cause overwriting of argument stack slots. // Potential tail calls could cause overwriting of argument stack slots.

bool isImmutable = !(getTargetMachine().Options.GuaranteedTailCallOpt && bool isImmutable = !(getTargetMachine().Options.GuaranteedTailCallOpt &&

(CallConv == CallingConv::Fast)); (CallConv == CallingConv::Fast));

unsigned PtrByteSize = 8; unsigned PtrByteSize = 8;

unsigned LinkageSize = Subtarget.getFrameLowering()->getLinkageSize(); unsigned LinkageSize = Subtarget.getFrameLowering()->getLinkageSize();

amykUnsubmitted

Not Done

Arg = DAG.getNode(ISD::ADD, dl, ArgOff.getValueType(), Arg, ArgOff);

- // Generate a shift to mode the element of interest to the front.

+ // Generate a shift to move the element of interest to the front.

SDValue SHRAmt = DAG.getConstant(Offset * 8, dl, MVT::i64);

Do you mean:

amyk: Do you mean:

static const MCPhysReg GPR[] = { static const MCPhysReg GPR[] = {

PPC::X3, PPC::X4, PPC::X5, PPC::X6, PPC::X3, PPC::X4, PPC::X5, PPC::X6,

PPC::X7, PPC::X8, PPC::X9, PPC::X10, PPC::X7, PPC::X8, PPC::X9, PPC::X10,

}; };

static const MCPhysReg VR[] = { static const MCPhysReg VR[] = {

PPC::V2, PPC::V3, PPC::V4, PPC::V5, PPC::V6, PPC::V7, PPC::V8, PPC::V2, PPC::V3, PPC::V4, PPC::V5, PPC::V6, PPC::V7, PPC::V8,

PPC::V9, PPC::V10, PPC::V11, PPC::V12, PPC::V13 PPC::V9, PPC::V10, PPC::V11, PPC::V12, PPC::V13

▲ Show 20 Lines • Show All 114 Lines • ▼ Show 20 Lines if (Flags.isByVal()) {

SDValue ArgOff = DAG.getConstant(PtrByteSize - ObjSize, dl, PtrVT); SDValue ArgOff = DAG.getConstant(PtrByteSize - ObjSize, dl, PtrVT);

Arg = DAG.getNode(ISD::ADD, dl, ArgOff.getValueType(), Arg, ArgOff); Arg = DAG.getNode(ISD::ADD, dl, ArgOff.getValueType(), Arg, ArgOff);

} }

InVals.push_back(Arg); InVals.push_back(Arg);

if (GPR_idx != Num_GPR_Regs) { if (GPR_idx != Num_GPR_Regs) {

unsigned VReg = MF.addLiveIn(GPR[GPR_idx++], &PPC::G8RCRegClass); unsigned VReg = MF.addLiveIn(GPR[GPR_idx++], &PPC::G8RCRegClass);

FuncInfo->addLiveInAttr(VReg, Flags); FuncInfo->addLiveInAttr(VReg, Flags);

SDValue Val = DAG.getCopyFromReg(Chain, dl, VReg, PtrVT); SDValue Val = DAG.getCopyFromReg(Chain, dl, VReg, PtrVT);

SDValue Store; EVT ObjType = EVT::getIntegerVT(*DAG.getContext(), ObjSize * 8);

leiUnsubmitted

Not Done

This case stmt basically does:

generate truc store for i8|i16|i32
push truc store to MemOps

I think it would be more clean to create a lambda that does both steps with MVT::i[8|16|32] as an arg and call it within the case stmts.

lei: This case stmt basically does: 1. generate truc store for i8|i16|i32 2. push truc store to…

nemanjaiUnsubmitted

Not Done

This seems unnecessarily complicated. There is already code in the legalizer that does this, so we should be able to just emit the store that we need.

If for whatever reason we can't, I would much rather not grow this already huge function by all this code but fold this into the new function.

nemanjai: This seems unnecessarily complicated. There is already code in the legalizer that does this, so…

SDValue Store =

amykUnsubmitted

Not Done

Question: I just wanted to check whether or not using Arg in cases 3/5/6/7 is expected, or if it is supposed to be frame index like in the code before?

amyk: Question: I just wanted to check whether or not using `Arg` in cases 3/5/6/7 is expected, or if…

stefanpAuthorUnsubmitted

Done

I think using Arg in this case is correct. We do want to make sure that we adjust the address in the Big Endian situation. I want to be able to treat the 3/5/6/7 case in a similar way to the way that we create the single load situations.

stefanp: I think using Arg in this case is correct. We do want to make sure that we adjust the address…

if (ObjSize==1 || ObjSize==2 || ObjSize==4) { DAG.getTruncStore(Val.getValue(1), dl, Val, Arg,

EVT ObjType = (ObjSize == 1 ? MVT::i8 :

(ObjSize == 2 ? MVT::i16 : MVT::i32));

Store = DAG.getTruncStore(Val.getValue(1), dl, Val, Arg,

MachinePointerInfo(&*FuncArg), ObjType); MachinePointerInfo(&*FuncArg), ObjType);

} else {

// For sizes that don't fit a truncating store (3, 5, 6, 7),

amykUnsubmitted

Not Done

Might be a silly question, but since it looks like the code for ObjSize == [1|2|4] looks like it is still the same, does it make sense for the switch to be in the else case for 3,5,6,7?
Or is it a preference to just transform the whole thing as a switch (like what you've done in the patch)? I'm just wondering as it kind of looks like the code in the cases are fairly repetitive for some of the cases.

amyk: Might be a silly question, but since it looks like the code for `ObjSize == [1|2|4]` looks like…

stefanpAuthorUnsubmitted

Done

It's just a style preference for me to add the switch statement to the whole thing. The code for sizes 1, 2, 4 has not changed and I've just replaced the:

EVT ObjType = (ObjSize == 1 ? MVT::i8 :
               (ObjSize == 2 ? MVT::i16 : MVT::i32));

with the switch statement.
I like this better because I have to check the size anyway to get the EVT so there is no point in splitting this part off into its own code. At least that is how I feel from a style perspective.

stefanp: It's just a style preference for me to add the switch statement to the whole thing. The code…

// store the whole register as-is to the parameter save area

// slot.

Store = DAG.getStore(Val.getValue(1), dl, Val, FIN,

MachinePointerInfo(&*FuncArg));

}

MemOps.push_back(Store); MemOps.push_back(Store);

} }

// Whether we copied from a register or not, advance the offset // Whether we copied from a register or not, advance the offset

// into the parameter save area by a full doubleword. // into the parameter save area by a full doubleword.

ArgOffset += PtrByteSize; ArgOffset += PtrByteSize;

continue; continue;

} }

// The value of the object is its address, which is the address of // The value of the object is its address, which is the address of

// its first stack doubleword. // its first stack doubleword.

InVals.push_back(FIN); InVals.push_back(FIN);

amykUnsubmitted

Not Done

nit: It looks like for cases 3 and 6, there are { } surrounding the case statements. It may be good to put them around 5 and 7 for consistency.

amyk: nit: It looks like for cases 3 and 6, there are `{ }` surrounding the case statements. It may…

stefanpAuthorUnsubmitted

Done

Actually, I'm going to remove the { } from all of the cases. I needed them when I was doing development but at this point they are not required.

stefanp: Actually, I'm going to remove the `{ }` from all of the cases. I needed them when I was doing…

// Store whatever pieces of the object are in registers to memory. // Store whatever pieces of the object are in registers to memory.

for (unsigned j = 0; j < ArgSize; j += PtrByteSize) { for (unsigned j = 0; j < ArgSize; j += PtrByteSize) {

if (GPR_idx == Num_GPR_Regs) if (GPR_idx == Num_GPR_Regs)

break; break;

unsigned VReg = MF.addLiveIn(GPR[GPR_idx], &PPC::G8RCRegClass); unsigned VReg = MF.addLiveIn(GPR[GPR_idx], &PPC::G8RCRegClass);

FuncInfo->addLiveInAttr(VReg, Flags); FuncInfo->addLiveInAttr(VReg, Flags);

▲ Show 20 Lines • Show All 13,418 Lines • Show Last 20 Lines

llvm/test/CodeGen/PowerPC/jaggedstructs.ll

	Show All 12 Lines
	%struct.S7 = type { [7 x i8] }			%struct.S7 = type { [7 x i8] }

	define void @test(%struct.S3* byval(%struct.S3) %s3, %struct.S5* byval(%struct.S5) %s5, %struct.S6* byval(%struct.S6) %s6, %struct.S7* byval(%struct.S7) %s7) nounwind {			define void @test(%struct.S3* byval(%struct.S3) %s3, %struct.S5* byval(%struct.S5) %s5, %struct.S6* byval(%struct.S6) %s6, %struct.S7* byval(%struct.S7) %s7) nounwind {
	entry:			entry:
	call void @check(%struct.S3* byval(%struct.S3) %s3, %struct.S5* byval(%struct.S5) %s5, %struct.S6* byval(%struct.S6) %s6, %struct.S7* byval(%struct.S7) %s7)			call void @check(%struct.S3* byval(%struct.S3) %s3, %struct.S5* byval(%struct.S5) %s5, %struct.S6* byval(%struct.S6) %s6, %struct.S7* byval(%struct.S7) %s7)
	ret void			ret void
	}			}

	; CHECK-DAG: std 3, 160(1)			; CHECK-LABEL: test
	; CHECK-DAG: std 6, 184(1)			; CHECK: stb 6, 191(1)
	; CHECK-DAG: std 5, 176(1)			; CHECK: rldicl 7, 6, 56, 8
	; CHECK-DAG: std 4, 168(1)			; CHECK: sth 7, 189(1)
				; CHECK: rldicl 6, 6, 40, 24
				; CHECK: stw 6, 185(1)
				; CHECK: sth 5, 182(1)
				; CHECK: rldicl 5, 5, 48, 16
				; CHECK: stw 5, 178(1)
				; CHECK: stb 4, 175(1)
				; CHECK: rldicl 4, 4, 56, 8
				; CHECK: stw 4, 171(1)
				; CHECK: stb 3, 167(1)
				; CHECK: rldicl 3, 3, 56, 8
				; CHECK: sth 3, 165(1)
	; CHECK-DAG: lbz {{[0-9]+}}, 167(1)			; CHECK-DAG: lbz {{[0-9]+}}, 167(1)
	; CHECK-DAG: lhz {{[0-9]+}}, 165(1)			; CHECK-DAG: lhz {{[0-9]+}}, 165(1)
	; CHECK-DAG: stb {{[0-9]+}}, 55(1)			; CHECK-DAG: stb {{[0-9]+}}, 55(1)
	; CHECK-DAG-DAG: sth {{[0-9]+}}, 53(1)			; CHECK-DAG-DAG: sth {{[0-9]+}}, 53(1)
	; CHECK-DAG: lbz {{[0-9]+}}, 175(1)			; CHECK-DAG: lbz {{[0-9]+}}, 175(1)
	; CHECK-DAG: lwz {{[0-9]+}}, 171(1)			; CHECK-DAG: lwz {{[0-9]+}}, 171(1)
	; CHECK-DAG: stb {{[0-9]+}}, 63(1)			; CHECK-DAG: stb {{[0-9]+}}, 63(1)
	; CHECK-DAG: stw {{[0-9]+}}, 59(1)			; CHECK-DAG: stw {{[0-9]+}}, 59(1)
	Show All 14 Lines

llvm/test/CodeGen/PowerPC/ppc64-byval-multi-store.ll

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py

nemanjaiUnsubmitted

Not Done

Please pre-commit to show only the differences in code-gen.

nemanjai: Please pre-commit to show only the differences in code-gen.

nemanjaiUnsubmitted

Not Done

Did you miss this comment or is the pre-commit and rebase coming?

nemanjai: Did you miss this comment or is the pre-commit and rebase coming?