[PATCH] D125700: [AMDGPU][GFX9] Support base+soffset+offset SMEM loads.

Thu May 19 05:28:16 PDT 2022

foad added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/SMInstructions.td:559
+  // (no-offset + soffset) case, namely:
+  //   imm=0  soffset_en=1  offset=?  soffset=<soffset>
+  //   imm=1  soffset_en=1  offset=0  soffset=<soffset>
----------------
kosarev wrote:
> dp wrote:
> > kosarev wrote:
> > > dp wrote:
> > > > This addressing mode is semantically equivalent to the one where soffset is encoded in the offset field. I'm not sure we really need it. Note that sp3 has no syntactic sugar to enforce this encoding. It may be useful for decoder, but I doubt it worth the trouble.
> > > Yes, these are not needed for codegen needs and may only be useful for disassembling. Should we leave the TODOs be until we know for sure what to do with this?
> > Let us see what other people here think. I'd have replaced TODOs with a description of limitations of the current design. But this is a matter of taste.
> Tagging @foad @arsenm @rampitec for visibility.
> 
> Re replacing TODOs with descriptions: my only concern here would be to avoid masking what might be considered a real problem.
I don't understand why these cases are not supported by the disassembler with the current patch. What happens if you try to disassemble them?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125700