This is an archive of the discontinued LLVM Phabricator instance.

Differential D15583

Reserve appropriate number of sgprs for flat scratch init.
ClosedPublic

Authored by cfang on Dec 16 2015, 12:10 PM.

Details

Reviewers
• tstellarAMD
Commits
rGcaaa3aa07c8f: AMDGPU/SI: Reserve appropriate number of sgprs for flat scratch init.
Summary

Reserve appropriate number of sgprs for flat scratch init.

Diff Detail

Event Timeline

cfang updated this revision to Diff 43047.Dec 16 2015, 12:10 PM
cfang retitled this revision from to For CI, reserve enough sgprs for flat scratch init if VCC is not used.
cfang updated this object.
cfang added a subscriber: llvm-commits.
Herald added a reviewer: • tstellarAMD. · View Herald TranscriptDec 16 2015, 12:10 PM
Herald added a subscriber: arsenm. · View Herald Transcript
• tstellarAMD added inline comments.Dec 16 2015, 12:27 PM
lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp
417–418

Might as well fix this for VI too.

This if above can stay the same but the if below should be incremented by 4 on VI and 2 on CI.

test/CodeGen/AMDGPU/ci-reserve-sgpr-for-flat-scratch-init.ll
1 ↗(On Diff #43047)

This test will become obsolete once we stop initializing the flat_scratch registers when we have no scratch usage.

Also, I don' t think there is really a good way to test this with normal IR, since the test is dependent on the register allocator always allocating the same number of registers.

I think it may be possible to test this with inline assembly. For now just drop the test, and I will try to add an inline assembly based test before I commit.

cfang updated this revision to Diff 43055.Dec 16 2015, 1:17 PM
cfang retitled this revision from For CI, reserve enough sgprs for flat scratch init if VCC is not used to Reserve appropriate number of sgprs for flat scratch init..
cfang updated this object.
cfang edited edge metadata.

update based on Tom's comments:

  1. also fix the sgpr reservation for VI+;
  2. drop the LIT test for now.
• tstellarAMD accepted this revision.Dec 17 2015, 8:43 AM
• tstellarAMD edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Dec 17 2015, 8:43 AM
cfang closed this revision.Jun 16 2016, 2:48 PM

Don't know why, but this should have been closed.

Revision Contents

PathSize
lib/
Target/
AMDGPU/
8 lines

Diff 43055

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

Context not available.
} }
} }
if (VCCUsed) if (VCCUsed || FlatUsed)
MaxSGPR += 2; MaxSGPR += 2;
tstellarAMDUnsubmitted
Not Done
ReplyInline Actions

Might as well fix this for VI too.

This if above can stay the same but the if below should be incremented by 4 on VI and 2 on CI.

tstellarAMD: Might as well fix this for VI too. This if above can stay the same but the if below should be…
if (FlatUsed) if (FlatUsed) {
MaxSGPR += 2; MaxSGPR += 2;
// 2 additional for VI+.
if (STM.getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
MaxSGPR += 2;
}
// We found the maximum register index. They start at 0, so add one to get the // We found the maximum register index. They start at 0, so add one to get the
// number of registers. // number of registers.
Context not available.