This change adds a real glc operand to the return atomic
instead of just string " glc" in the middle of the asm
string.
Improves asm parser diagnostics.
Paths
| Differential D90730
[AMDGPU] Add default 1 glc operand to rtn atomics ClosedPublic Authored by rampitec on Nov 3 2020, 4:25 PM.
Details
Summary This change adds a real glc operand to the return atomic Improves asm parser diagnostics.
Diff Detail Event Timelinerampitec retitled this revision from [AMDGPU] Add default 1 glc operand to flat rtn atomics to [AMDGPU] Add default 1 glc operand to rtn atomics. Comment ActionsAdded the same change to MUBUF. Comment Actions LGTM with a minor issue
This revision is now accepted and ready to land.Nov 5 2020, 4:07 AM scott.linder added inline comments.
rampitec added inline comments.
Closed by commit rGf738aee0bbf3: [AMDGPU] Add default 1 glc operand to rtn atomics (authored by rampitec). · Explain WhyNov 5 2020, 10:42 AM This revision was automatically updated to reflect the committed changes. rampitec marked an inline comment as done.
Revision Contents
Diff 302711 llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
llvm/lib/Target/AMDGPU/FLATInstructions.td
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
llvm/lib/Target/AMDGPU/SIInstrInfo.td
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-amdgpu-atomic-cmpxchg-flat.mir
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-amdgpu-atomic-cmpxchg-global.mir
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-atomicrmw-add-flat.mir
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-atomicrmw-add-global.mir
llvm/test/CodeGen/AMDGPU/break-vmem-soft-clauses.mir
llvm/test/CodeGen/AMDGPU/fp-atomic-to-s_denormmode.mir
llvm/test/CodeGen/AMDGPU/memory_clause.mir
llvm/test/CodeGen/AMDGPU/vmem-to-salu-hazard.mir
llvm/test/CodeGen/AMDGPU/waitcnt-vscnt.mir
llvm/test/MC/AMDGPU/flat-gfx10.s
llvm/test/MC/AMDGPU/flat-gfx9.s
llvm/test/MC/AMDGPU/gfx1030_new.s
|
Is the _1 variant added here rather than replacing the existing one to support an old syntax, or to support disjoint classes of instructions which need different defaults/validation, or something else? I'm not sure I understand, but renaming the _1 to something more descriptive or adding a comment might help. It also seems like the other isX functions are all checking for something distinct (i.e. there is ImmTyOffset, ImmTyOffset0, and ImmTyOffset1 above), is that not needed here?