This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][NFC] Rename ReserveTrapVGPRs -> ReserveRegs
ClosedPublic

Authored by kzhuravl on May 9 2016, 1:52 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

kzhuravl updated this revision to Diff 56623.May 9 2016, 1:52 PM
kzhuravl retitled this revision from to [AMDGPU] Only allow register reservation for debugger usage with -O0 + minor renaming as discussed.
kzhuravl updated this object.
kzhuravl added reviewers: tstellarAMD, arsenm.
kzhuravl added a subscriber: llvm-commits.
kzhuravl updated this revision to Diff 56750.May 10 2016, 10:50 AM
kzhuravl retitled this revision from [AMDGPU] Only allow register reservation for debugger usage with -O0 + minor renaming as discussed to [AMDGPU] Only allow register reservation for debugger usage with -g and -O0 + minor renaming.

Emit error if debug info is not present

arsenm added inline comments.May 11 2016, 12:38 PM
lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
140–143 ↗(On Diff #56750)

I don't see why this would be an error, particularly a backend error. Talking about -g here doesn't make sense

kzhuravl added inline comments.May 11 2016, 12:48 PM
lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
140–143 ↗(On Diff #56750)

We want to restrict additional debugger features (register reservation, nops insertion, etc.) to be only used with debug info present and -O0 opt level.

I can change the error message to say "only allowed with debug info present".

Should we use assert instead? What are your thoughts?

arsenm added inline comments.May 11 2016, 1:17 PM
lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
140–143 ↗(On Diff #56750)

There should be no error. It should proceed and emit the program. There's no reason the registers can't be reserved with optimizations. The nop insertion will be less useful with optimizations, but should still work. Telling the user that they want to disable optimizations with debugging is a frontend question

kzhuravl marked 3 inline comments as done.May 11 2016, 1:53 PM
kzhuravl updated this revision to Diff 56962.May 11 2016, 2:24 PM
kzhuravl retitled this revision from [AMDGPU] Only allow register reservation for debugger usage with -g and -O0 + minor renaming to [AMDGPU][NFC] Rename ReserveTrapVGPRs -> ReserveRegs.

Address Matt's comments

arsenm accepted this revision.May 23 2016, 9:25 AM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 23 2016, 9:25 AM
This revision was automatically updated to reflect the committed changes.