[PATCH] D125117: [AMDGPU][GFX10] Support base+soffset+offset SMEM loads.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SMInstructions.td:751
 
-  let Inst{7-0}   = !if(ps.has_offset, offset{7-0}, ?);
-  let Inst{8}     = imm;
+  let Inst{7-0}   = !if(ps.has_offset, offset{7-0}, !if(ps.has_soffset, soffset{7-0}, ?));
+  let Inst{8}     = ps.has_offset;
----------------
Can you just `soffset` instead of `soffset{7-0}`, here and elsewhere?


================
Comment at: llvm/lib/Target/AMDGPU/SMInstructions.td:911-913
+  let Inst{52-32} = !if(ps.has_offset, offset{20-0}, !if(ps.has_soffset, 0, ?));
+  let Inst{63-57} = !if(ps.has_soffset, soffset{6-0},
+                        !if(ps.has_offset, !cast<int>(SGPR_NULL.HWEncoding), ?));
----------------
These expressions seem over-complicated. Is it really important to set these fields to `?` in the "!has_offset && !has_soffset" case, which presumably will never happen? Can you simplify to:
```
  let Inst{52-32} = !if(ps.has_offset, offset{20-0}, 0);
  let Inst{63-57} = !if(ps.has_soffset, soffset{6-0}, !cast<int>(SGPR_NULL.HWEncoding));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125117