[PATCH] D126207: [AMDGPU][MC][GFX11] Support base+soffset+offset SMEM loads.

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SMInstructions.td:1145
+    op, ps#"_SGPR_IMM", opName, (ins SReg_32:$soffset, smem_offset_mod:$offset)>;
+  def : MnemonicAlias<!cast<SM_Pseudo>(ps#"_COMMON").Mnemonic, opName>,
+                      Requires<[isGFX11Plus]>;
----------------
kosarev wrote:
> dp wrote:
> > Is the _COMMON variant defined for aesthetic reasons? I believe the following code should work:
> > 
> >     def : MnemonicAlias<!cast<SM_Load_Pseudo>(ps#"_IMM").Mnemonic, opName>,
> > 
> The aim here is to make the code reflect the fact that the pseudos share the same mnemonic (and I'm not sure casting to `SM_Load_Pseudo` instead of `SM_Pseudo` makes it obvious enough). This introduces a pseudo that is not used as such, but that seems a result of using pseudos to describe instruction properties -- a problem that we might want to address separately at some point.
This is a matter of taste, but for me the _COMMON definition does not make code clearer because the purpose of this definition is not immediately obvious. Maybe the name is a bit misleading and something like _MNEMO would be better?

Another (more important) problem is that the _COMMON definition if not for free - it contaminates compiler tables with unused entries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126207