This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Revert wide LDS DMA support.
ClosedPublic

Authored by rampitec on May 11 2022, 1:14 PM.

Details

Summary

This reverts ffbee7acdcaaf, see also bug 37653 which it was fixing.
The bug claims this is an undocumented feature which actually works.
In the reality it is documented as not working for a good reason.
It likely does something, but it is useless anyway. These instructions
write into the LDS. The LDS address is:

M0 + inst_offset + (TIDinWave * 4).

For a store wider than a DWORD neighboring lanes will overwrite each
other.

Diff Detail

Event Timeline

rampitec created this revision.May 11 2022, 1:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2022, 1:14 PM
rampitec requested review of this revision.May 11 2022, 1:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2022, 1:14 PM
Herald added a subscriber: wdng. · View Herald Transcript
dp accepted this revision.May 11 2022, 1:23 PM

LGTM

This revision is now accepted and ready to land.May 11 2022, 1:23 PM
arsenm added inline comments.May 13 2022, 2:50 AM
llvm/test/MC/AMDGPU/mubuf.s
850

If it actually does something, shouldn't the assembler/disassembler still recognize it? We just wouldn't continue selecting

foad added inline comments.May 13 2022, 3:17 AM
llvm/test/MC/AMDGPU/mubuf.s
850

All the documentation I've seen says you're not allowed to use the lds bit on larger-than-dword loads. Then again, this all comes from https://bugs.llvm.org/show_bug.cgi?id=37653 which says it is a "potentially useful but still undocumented feature"...

foad accepted this revision.May 13 2022, 3:18 AM

LGTM - the fact that it's still undocumented and never turned out to be "potentially useful" is good enough for me.

rampitec added inline comments.May 13 2022, 9:23 AM
llvm/test/MC/AMDGPU/mubuf.s
850

Right, all the documentation says it is not allowed. For example https://developer.amd.com/wp-content/resources/Vega_Shader_ISA_28July2017.pdf, 8.1.9 Memory Buffer Load to LDS:

The MUBUF instruction format allows reading data from a memory buffer directly into LDS
without passing through VGPRs. This is supported for the following subset of MUBUF
instructions.
• BUFFER_LOAD_{ubyte, sbyte, ushort, sshort, dword, format_x}.

For a wider operands it does something, but that something data race on store to LDS and corrupted output.

This revision was landed with ongoing or failed builds.May 16 2022, 11:23 AM
This revision was automatically updated to reflect the committed changes.