[llvm] [TableGen][AMDGPU][AsmParser] Catch and fix ambiguous instructions (PR #211093)

[Jay Foad via llvm-commits]

jayfoad wrote:

Passing on some correctness concerns from an AI code review. I have not verified these but they at least sound plausible to me:

1. GFX9 region (GDS) atomics now emit a GFX8-only instruction (DSInstructions.td:1896). The PR restricts the base VI DS real to isGFX8Only. But on gfx900 with GDS, the patterns at DSInstructions.td:535-572 still select the base pseudo, e.g. DS_ADD_RTN_U32 with gds=1 and M0. pseudoToMCOpcode then falls back to the VI family and emits DS_ADD_RTN_U32_vi, whose predicate now says GFX8-only. Nothing fails today only because verifyInstructionPredicates in AMDGPUMCInstLower.cpp is commented out.
2. Two GFX13 exclusions don't actually hold (AMDGPU.td:3124). isGFX13Only and isGFX13Plus are declared mutually exclusive with isGFX12Not12_50, which is (GFX12Insts and not GFX1250Insts). FeatureGFX13 doesn't imply FeatureGFX1250Insts; only the gfx1300 processor definition adds it. So -mcpu=gfx1300 -mattr=-gfx1250-insts satisfies both predicates, and a real ambiguity would be hidden. The existing isGFX12PlusNot12_50 predicates treat "GFX13 without 1250 insts" as a possible configuration.
3. Storing a Twine in a local variable (AsmMatcherEmitter.cpp:3492, Twine Msg = ...). Twine.h forbids this. It works today only because the two pieces get folded into the result. Adding a third piece would leave it pointing at destroyed temporaries. Use std::string Msg =  (...).str() instead.


https://github.com/llvm/llvm-project/pull/211093