This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Invert partial vgpr to agpr spill lane order
ClosedPublic

Authored by rampitec on Aug 25 2021, 2:46 PM.

Details

Summary

On targets requiring VGPR alignment we may end up spilling an
unaligned register if we were partially spilled odd number of
leading lanes. The reminder will start with an odd register.

This problem is solved by inverting the order of lanes to
be spillied so that we start from the end.

Diff Detail

Unit TestsFailed

Event Timeline

rampitec created this revision.Aug 25 2021, 2:46 PM
rampitec requested review of this revision.Aug 25 2021, 2:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 25 2021, 2:46 PM
Herald added a subscriber: wdng. · View Herald Transcript

I don't understand what the problem is. The spilling is all done with 32-bit pieces, so why does the alignment matter?

I don't understand what the problem is. The spilling is all done with 32-bit pieces, so why does the alignment matter?

Say we need to spill v[0:3] like in the added test and we have one agpr available. Current logic will spill v0 into agpr and v[1:3] into scratch.

With mubuf there is no problem, we will issue 3 buffer_store instructions, but with flat scratch it will be a single store of an unaligned register.

arsenm added inline comments.Aug 25 2021, 3:36 PM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
1189

Should add a comment explaining why this is done in reverse

rampitec updated this revision to Diff 368767.Aug 25 2021, 3:54 PM
rampitec marked an inline comment as done.

Added comment to explain reverse allocation order.
Fixed relevant clang-format complaints.

arsenm accepted this revision.Aug 25 2021, 4:24 PM
This revision is now accepted and ready to land.Aug 25 2021, 4:24 PM
foad added inline comments.Aug 26 2021, 12:51 AM
llvm/test/CodeGen/AMDGPU/spill-to-agpr-partiall.mir
1

Typo in filename "partiall"

rampitec updated this revision to Diff 368907.Aug 26 2021, 9:19 AM
rampitec marked an inline comment as done.

Renamed test.

This revision was landed with ongoing or failed builds.Aug 26 2021, 9:39 AM
This revision was automatically updated to reflect the committed changes.