This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix copyPhysReg to not produce unalined vgpr access
ClosedPublic

Authored by rampitec on Mar 12 2021, 1:26 PM.

Details

Summary

RA can insert something like a sub1_sub2 COPY of a wide VGPR
tuple which results in the unaligned acces with v_pk_mov_b32
after the copy is expanded. This is regression after D97316.

Diff Detail

Event Timeline

rampitec created this revision.Mar 12 2021, 1:26 PM
rampitec requested review of this revision.Mar 12 2021, 1:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 12 2021, 1:26 PM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm added inline comments.Mar 12 2021, 2:17 PM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
879

It would be easier to follow if you directly referred to the aligned class. getVGPR64Class will always return that anyway on the targets with the alignment requirement

914–916

This seems more complicated than it needs to be

rampitec added inline comments.Mar 12 2021, 2:19 PM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
879

That's exactly the point, to get either aligned or unaligned class depending on ST.

914–916

Suggestions?

rampitec added inline comments.Mar 12 2021, 2:39 PM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
914–916

I.e. I do not see an easy way to check an RC is aligned.

rampitec updated this revision to Diff 330385.Mar 12 2021, 3:22 PM

Removed some now unneeded RC checks.

rampitec marked an inline comment as done.Mar 12 2021, 3:22 PM
arsenm added inline comments.Mar 15 2021, 11:21 AM
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
914–916

Probably should add a utility function to check this. The verifier has a similarly confusing check here:

if (!RC || ((IsVGPR && !RC->hasSuperClassEq(RI.getVGPRClassForBitWidth(
                             RI.getRegSizeInBits(*RC)))) ||
rampitec updated this revision to Diff 330755.Mar 15 2021, 12:02 PM
rampitec marked an inline comment as done.

Added helper SIRegisterInfo::isProperlyAlignedRC().

arsenm accepted this revision.Mar 15 2021, 1:58 PM
This revision is now accepted and ready to land.Mar 15 2021, 1:58 PM
This revision was landed with ongoing or failed builds.Mar 15 2021, 2:14 PM
This revision was automatically updated to reflect the committed changes.