This is an archive of the discontinued LLVM Phabricator instance.

Differential D95700

[AMDGPU] Be more specific in needsFrameBaseReg
ClosedPublic

Authored by rampitec on Jan 29 2021, 2:40 PM.

Details

Reviewers
arsenm
sebastian-ne
Commits
rG9dbe736cbd2c: [AMDGPU] Be more specific in needsFrameBaseReg
Summary

A condition "mayLoadOrStore" is too broad for that function.

Diff Detail

Event Timeline

rampitec created this revision.Jan 29 2021, 2:40 PM
Herald added subscribers: kerbowa, hiraditya, t-tye and 6 others. · View Herald TranscriptJan 29 2021, 2:40 PM
rampitec requested review of this revision.Jan 29 2021, 2:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 29 2021, 2:40 PM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm added inline comments.Jan 29 2021, 2:42 PM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
416–417

Move this up and then just exit on isFLATScratch

rampitec added inline comments.Jan 29 2021, 2:45 PM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
416–417

I would need to call getScratchInstrOffset() twice.

arsenm accepted this revision.Jan 29 2021, 2:46 PM
This revision is now accepted and ready to land.Jan 29 2021, 2:46 PM
Closed by commit rG9dbe736cbd2c: [AMDGPU] Be more specific in needsFrameBaseReg (authored by rampitec). · Explain WhyJan 29 2021, 2:53 PM
This revision was automatically updated to reflect the committed changes.
rampitec added a commit: rG9dbe736cbd2c: [AMDGPU] Be more specific in needsFrameBaseReg.

Revision Contents

PathSize
llvm/
lib/
Target/
AMDGPU/
2 lines

Diff 320223

llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp

Show First 20 Lines • Show All 402 Lines • ▼ Show 20 Lines assert((Idx == AMDGPU::getNamedOperandIdx(MI->getOpcode(),
(Idx == AMDGPU::getNamedOperandIdx(MI->getOpcode(), (Idx == AMDGPU::getNamedOperandIdx(MI->getOpcode(),
AMDGPU::OpName::saddr))) && AMDGPU::OpName::saddr))) &&
"Should never see frame index on non-address operand"); "Should never see frame index on non-address operand");
return getScratchInstrOffset(MI); return getScratchInstrOffset(MI);
} }
bool SIRegisterInfo::needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const { bool SIRegisterInfo::needsFrameBaseReg(MachineInstr *MI, int64_t Offset) const {
if (!MI->mayLoadOrStore()) if (!SIInstrInfo::isMUBUF(*MI) && !SIInstrInfo::isFLATScratch(*MI))
return false; return false;
int64_t FullOffset = Offset + getScratchInstrOffset(MI); int64_t FullOffset = Offset + getScratchInstrOffset(MI);
if (SIInstrInfo::isMUBUF(*MI)) if (SIInstrInfo::isMUBUF(*MI))
return !SIInstrInfo::isLegalMUBUFImmOffset(FullOffset); return !SIInstrInfo::isLegalMUBUFImmOffset(FullOffset);
arsenmUnsubmitted
Not Done
ReplyInline Actions

Move this up and then just exit on isFLATScratch

arsenm: Move this up and then just exit on isFLATScratch
rampitecAuthorUnsubmitted
Done
ReplyInline Actions

I would need to call getScratchInstrOffset() twice.

rampitec: I would need to call getScratchInstrOffset() twice.
const SIInstrInfo *TII = ST.getInstrInfo(); const SIInstrInfo *TII = ST.getInstrInfo();
return !TII->isLegalFLATOffset(FullOffset, AMDGPUAS::PRIVATE_ADDRESS, true); return !TII->isLegalFLATOffset(FullOffset, AMDGPUAS::PRIVATE_ADDRESS, true);
} }
Register SIRegisterInfo::materializeFrameBaseRegister(MachineBasicBlock *MBB, Register SIRegisterInfo::materializeFrameBaseRegister(MachineBasicBlock *MBB,
int FrameIdx, int FrameIdx,
int64_t Offset) const { int64_t Offset) const {
▲ Show 20 Lines • Show All 1,840 Lines • Show Last 20 Lines