This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Validate GDS in the assembler
ClosedPublic

Authored by foad on Aug 4 2023, 7:41 AM.

Details

Reviewers
arsenm
Group Reviewers
Restricted Project
Commits
rG18919ee75954: [AMDGPU] Validate GDS in the assembler
Summary

GFX90A DS instructions cannot use the gds modifier, except for the
DS_GWS_* instructions where it is still mandatory.

Diff Detail

Event Timeline

foad created this revision.Aug 4 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 7:41 AM
foad requested review of this revision.Aug 4 2023, 7:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 7:41 AM

Note that the disassembler will still happily decode the gds bit. I hope that's OK. The disassembler has always been more forgiving than the assembler.

I think disasm test lines with gds shall also be removed, even if we still decode and print it.

Note that the disassembler will still happily decode the gds bit. I hope that's OK. The disassembler has always been more forgiving than the assembler.

I think the disassembler should continue printing it, the disassembler needs to be more tolerant of anything that it sees

kosarev added inline comments.Aug 7 2023, 3:19 AM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
4428

Are there tests that check triggering this message?

foad updated this revision to Diff 547738.Aug 7 2023, 5:26 AM

Update.

foad marked an inline comment as done.Aug 7 2023, 5:28 AM

I think disasm test lines with gds shall also be removed, even if we still decode and print it.

I removed those lines and added a single test that the disassembler still decodes gds. Is that a reasonable compromise?

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
4428

Good point. There were not. I added one.

arsenm accepted this revision.Aug 10 2023, 2:52 PM
This revision is now accepted and ready to land.Aug 10 2023, 2:52 PM
This revision was automatically updated to reflect the committed changes.
foad marked an inline comment as done.