This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Make temp vgpr selection stable in indirectCopyToAGPR
ClosedPublic

Authored by rampitec on Jun 10 2022, 12:01 PM.

Details

Summary

This uses rotating reminder of division by 3 to select another
temp vgpr each next time in a sequence of several agpr copies.
Therefore, temp vgpr selection depends on the generated agpr
number. This number could change with any unrelated change to
the register definitions.

Stabilize the selection by using a real agpr number.

Diff Detail

Event Timeline

rampitec created this revision.Jun 10 2022, 12:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2022, 12:01 PM
rampitec requested review of this revision.Jun 10 2022, 12:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2022, 12:01 PM
Herald added a subscriber: wdng. · View Herald Transcript
foad accepted this revision.Jun 13 2022, 1:45 AM

Thanks! This also cropped up in D119552.

This revision is now accepted and ready to land.Jun 13 2022, 1:45 AM
cdevadas added inline comments.Jun 13 2022, 5:49 AM
llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
528–529

Unrelated to this patch; The highest VGPR register number should be shifted back to the lowest free register.
Was there any follow-up patch planned?

This revision was landed with ongoing or failed builds.Jun 13 2022, 9:40 AM
This revision was automatically updated to reflect the committed changes.
rampitec added inline comments.Jun 13 2022, 11:22 AM
llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
528–529

Ping @hsmhsm for this question.

hsmhsm added inline comments.Jun 14 2022, 4:28 AM
llvm/test/CodeGen/AMDGPU/accvgpr-copy.mir
528–529

We need to see - for this testcase, the code which handles the finding and reserving lowest available free register will be hit or not. If the nature of this testcase is like that we won't hit that part of the code, then this is indeed the expected behavior. Otherwise, there is an unhandled bug in the patch https://reviews.llvm.org/D123525 which need to be fixed.