[PATCH] D128836: [AMDGPU][GlobalISel] Support register offsets for SMRDs.

Ivan Kosarev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 05:38:33 PDT 2022


kosarev marked an inline comment as done.
kosarev added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3225
 
   if (mi_match(Def->getOperand(2).getReg(), MRI, m_ZeroInt())) {
     return Def->getOperand(1).getReg();
----------------
foad wrote:
> Not your fault, but this seems to be missing checks (or asserts) on the number of operands and the type of the operands of the G_MERGE_VALUES.
Indeed. Will address this with a separate patch.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3825
   // FIXME: We should shrink the GEP if the offset is known to be <= 32-bits,
-  // then we can select all ptr + 32-bit offsets not just immediate offsets.
-  if (AddrInfo.empty() || AddrInfo[0].SgprParts.size() != 1)
+  // then we can select all ptr + 32-bit offsets.
+  if (AddrInfo.empty())
----------------
foad wrote:
> I don't understand this comment. What cases are we still missing?
I guess this refers to i64 values that are known to fit 32 bits, such as `(x & 3) << 2`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128836



More information about the llvm-commits mailing list