This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Remove vdata from buffer to lds load
ClosedPublic

Authored by rampitec on Apr 26 2022, 3:55 PM.

Diff Detail

Event Timeline

rampitec created this revision.Apr 26 2022, 3:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2022, 3:55 PM
rampitec requested review of this revision.Apr 26 2022, 3:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2022, 3:55 PM
Herald added a subscriber: wdng. · View Herald Transcript

A fake def seems like a bug to me for which I have the only excuse: no one really used it. This might be OK in asm, but certainty not in the MIR instructions stream.

This changes asm syntax though.

Did these instructions just not actually have vdata?

llvm/lib/Target/AMDGPU/BUFInstructions.td
497

Extra space after outs

Did these instructions just not actually have vdata?

Yes, they don't. These are DMA. They read vmem and write LDS without touching any VGPRs. That's the whole point for these to exist.

buffer_store_lds does not have it, any of flat DMA instructions don't. This is just the old bug.

llvm/lib/Target/AMDGPU/BUFInstructions.td
497

Did we finally fixed the issue when this extra space was needed?

arsenm accepted this revision.Apr 26 2022, 5:05 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/BUFInstructions.td
497

Not sure

This revision is now accepted and ready to land.Apr 26 2022, 5:05 PM
rampitec updated this revision to Diff 425352.Apr 26 2022, 5:17 PM
rampitec marked 2 inline comments as done.
rampitec added inline comments.
llvm/lib/Target/AMDGPU/BUFInstructions.td
497

Apparently we did. Removed space.

This revision was landed with ongoing or failed builds.Apr 26 2022, 5:28 PM
This revision was automatically updated to reflect the committed changes.
foad added inline comments.
llvm/test/MC/AMDGPU/gfx7_asm_mubuf.s
102

Can you explain this? Does the lds version of this instruction not exist at allon gfx7? Or does it just need different operands?

rampitec added inline comments.Apr 27 2022, 2:14 AM
llvm/test/MC/AMDGPU/gfx7_asm_mubuf.s
102

It does not exist.