Skip to content

Commit

Permalink
[AMDGPU] Don't create MachinePointerInfos with an UndefValue pointer
Browse files Browse the repository at this point in the history
Summary:
The only useful information the UndefValue conveys is the address space,
which MachinePointerInfo can represent directly without referring to an
IR value.

Reviewers: arsenm, rampitec

Subscribers: kzhuravl, jvesely, wdng, nhaehnle, yaxunl, dstuttard, tpr, t-tye, hiraditya, Petar.Avramovic, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D71838
  • Loading branch information
jayfoad committed Dec 23, 2019
1 parent 5b1d0dc commit c7c05b0
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 114 deletions.
3 changes: 1 addition & 2 deletions llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,7 @@ void AMDGPUCallLowering::lowerParameter(MachineIRBuilder &B,
MachineFunction &MF = B.getMF();
const Function &F = MF.getFunction();
const DataLayout &DL = F.getParent()->getDataLayout();
PointerType *PtrTy = PointerType::get(ParamTy, AMDGPUAS::CONSTANT_ADDRESS);
MachinePointerInfo PtrInfo(UndefValue::get(PtrTy));
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
unsigned TypeSize = DL.getTypeStoreSize(ParamTy);
Register PtrReg = lowerParameterPtr(B, ParamTy, Offset);

Expand Down
8 changes: 2 additions & 6 deletions llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1186,12 +1186,8 @@ Register AMDGPULegalizerInfo::getSegmentAperture(
// private_segment_aperture_base_hi.
uint32_t StructOffset = (AS == AMDGPUAS::LOCAL_ADDRESS) ? 0x40 : 0x44;

// FIXME: Don't use undef
Value *V = UndefValue::get(PointerType::get(
Type::getInt8Ty(MF.getFunction().getContext()),
AMDGPUAS::CONSTANT_ADDRESS));

MachinePointerInfo PtrInfo(V, StructOffset);
// TODO: can we be smarter about machine pointer info?
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
MachineMemOperand *MMO = MF.getMachineMemOperand(
PtrInfo,
MachineMemOperand::MOLoad |
Expand Down
12 changes: 3 additions & 9 deletions llvm/lib/Target/AMDGPU/R600ISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1175,8 +1175,7 @@ SDValue R600TargetLowering::lowerPrivateTruncStore(StoreSDNode *Store,

// Load dword
// TODO: can we be smarter about machine pointer info?
MachinePointerInfo PtrInfo(UndefValue::get(
Type::getInt32PtrTy(*DAG.getContext(), AMDGPUAS::PRIVATE_ADDRESS)));
MachinePointerInfo PtrInfo(AMDGPUAS::PRIVATE_ADDRESS);
SDValue Dst = DAG.getLoad(MVT::i32, DL, Chain, Ptr, PtrInfo);

Chain = Dst.getValue(1);
Expand Down Expand Up @@ -1406,8 +1405,7 @@ SDValue R600TargetLowering::lowerPrivateExtLoad(SDValue Op,

// Load dword
// TODO: can we be smarter about machine pointer info?
MachinePointerInfo PtrInfo(UndefValue::get(
Type::getInt32PtrTy(*DAG.getContext(), AMDGPUAS::PRIVATE_ADDRESS)));
MachinePointerInfo PtrInfo(AMDGPUAS::PRIVATE_ADDRESS);
SDValue Read = DAG.getLoad(MVT::i32, DL, Chain, Ptr, PtrInfo);

// Get offset within the register.
Expand Down Expand Up @@ -1608,9 +1606,6 @@ SDValue R600TargetLowering::LowerFormalArguments(
continue;
}

PointerType *PtrTy = PointerType::get(VT.getTypeForEVT(*DAG.getContext()),
AMDGPUAS::PARAM_I_ADDRESS);

// i64 isn't a legal type, so the register type used ends up as i32, which
// isn't expected here. It attempts to create this sextload, but it ends up
// being invalid. Somehow this seems to work with i64 arguments, but breaks
Expand All @@ -1631,11 +1626,10 @@ SDValue R600TargetLowering::LowerFormalArguments(
// XXX - I think PartOffset should give you this, but it seems to give the
// size of the register which isn't useful.

unsigned ValBase = ArgLocs[In.getOrigArgIndex()].getLocMemOffset();
unsigned PartOffset = VA.getLocMemOffset();
unsigned Alignment = MinAlign(VT.getStoreSize(), PartOffset);

MachinePointerInfo PtrInfo(UndefValue::get(PtrTy), PartOffset - ValBase);
MachinePointerInfo PtrInfo(AMDGPUAS::PARAM_I_ADDRESS);
SDValue Arg = DAG.getLoad(
ISD::UNINDEXED, Ext, VT, DL, Chain,
DAG.getConstant(PartOffset, DL, MVT::i32), DAG.getUNDEF(MVT::i32),
Expand Down
10 changes: 2 additions & 8 deletions llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -578,10 +578,7 @@ void SIFrameLowering::emitEntryFunctionScratchSetup(const GCNSubtarget &ST,

// We now have the GIT ptr - now get the scratch descriptor from the entry
// at offset 0 (or offset 16 for a compute shader).
PointerType *PtrTy =
PointerType::get(Type::getInt64Ty(MF.getFunction().getContext()),
AMDGPUAS::CONSTANT_ADDRESS);
MachinePointerInfo PtrInfo(UndefValue::get(PtrTy));
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
const MCInstrDesc &LoadDwordX4 = TII->get(AMDGPU::S_LOAD_DWORDX4_IMM);
auto MMO = MF.getMachineMemOperand(PtrInfo,
MachineMemOperand::MOLoad |
Expand Down Expand Up @@ -623,10 +620,7 @@ void SIFrameLowering::emitEntryFunctionScratchSetup(const GCNSubtarget &ST,
} else {
const MCInstrDesc &LoadDwordX2 = TII->get(AMDGPU::S_LOAD_DWORDX2_IMM);

PointerType *PtrTy =
PointerType::get(Type::getInt64Ty(MF.getFunction().getContext()),
AMDGPUAS::CONSTANT_ADDRESS);
MachinePointerInfo PtrInfo(UndefValue::get(PtrTy));
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
auto MMO = MF.getMachineMemOperand(PtrInfo,
MachineMemOperand::MOLoad |
MachineMemOperand::MOInvariant |
Expand Down
12 changes: 3 additions & 9 deletions llvm/lib/Target/AMDGPU/SIISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1473,9 +1473,7 @@ SDValue SITargetLowering::lowerKernargMemParameter(
const SDLoc &SL, SDValue Chain,
uint64_t Offset, unsigned Align, bool Signed,
const ISD::InputArg *Arg) const {
Type *Ty = MemVT.getTypeForEVT(*DAG.getContext());
PointerType *PtrTy = PointerType::get(Ty, AMDGPUAS::CONSTANT_ADDRESS);
MachinePointerInfo PtrInfo(UndefValue::get(PtrTy));
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);

// Try to avoid using an extload by loading earlier than the argument address,
// and extracting the relevant bits. The load should hopefully be merged with
Expand Down Expand Up @@ -2875,8 +2873,7 @@ SDValue SITargetLowering::LowerCall(CallLoweringInfo &CLI,
Chain, DL, DstAddr, Arg, SizeNode, Outs[i].Flags.getByValAlign(),
/*isVol = */ false, /*AlwaysInline = */ true,
/*isTailCall = */ false, DstInfo,
MachinePointerInfo(UndefValue::get(Type::getInt8PtrTy(
*DAG.getContext(), AMDGPUAS::PRIVATE_ADDRESS))));
MachinePointerInfo(AMDGPUAS::PRIVATE_ADDRESS));

MemOpChains.push_back(Cpy);
} else {
Expand Down Expand Up @@ -4717,10 +4714,7 @@ SDValue SITargetLowering::getSegmentAperture(unsigned AS, const SDLoc &DL,
// TODO: Use custom target PseudoSourceValue.
// TODO: We should use the value from the IR intrinsic call, but it might not
// be available and how do we get it?
Value *V = UndefValue::get(PointerType::get(Type::getInt8Ty(*DAG.getContext()),
AMDGPUAS::CONSTANT_ADDRESS));

MachinePointerInfo PtrInfo(V, StructOffset);
MachinePointerInfo PtrInfo(AMDGPUAS::CONSTANT_ADDRESS);
return DAG.getLoad(MVT::i32, DL, QueuePtr.getValue(1), Ptr, PtrInfo,
MinAlign(64, StructOffset),
MachineMemOperand::MODereferenceable |
Expand Down
Loading

0 comments on commit c7c05b0

Please sign in to comment.