[PATCH] D139412: [AMDGPU] Change handling of s_endpgm's optional operand. NFC.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SOPInstructions.td:1942
 
-multiclass SOPP_Real_32_gfx11<bits<7> op, string real_name = !cast<SOPP_Pseudo>(NAME).Mnemonic # " "> {
-  def _gfx11 : SOPP_Real_32<op, !cast<SOPP_Pseudo>(NAME), real_name>,
+multiclass SOPP_Real_32_gfx11<bits<7> op> {
+  def _gfx11 : SOPP_Real_32<op, !cast<SOPP_Pseudo>(NAME), !cast<SOPP_Pseudo>(NAME).Mnemonic>,
----------------
dp wrote:
> foad wrote:
> > @dp what do you think of this patch? Personally I like it because it simplifies all these SOPP_Real multiclasses, by removing all the machinery for passing around and overriding real_name. But I am open to other opinions.
> Your patch is definitely an improvement.
> 
> After looking at the original code I noticed that SOP opcodes without operands have a space after mnemonic. I only checked `s_endpgm_saved` and `s_set_gpr_idx_off` but it looks like other opcodes w/o operands have a similar issue. Your patch seems to address this issue though it is not mentioned in the description.
> 
> Most definitions use `SOPP_Pseudo` with an operand starting from a space. Shouldn't `SOPP_Pseudo` add a space automatically? This looks feasible; `endpgm` case may be handled by a subclass of `SOPP_Pseudo`.
I only checked endpgm because there is a special strict-whitespace test for it: `test/MC/AMDGPU/s_endpgm.s`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139412/new/

https://reviews.llvm.org/D139412