[PATCH] D81524: AMDGPU/GlobalISel: Fix 8-byte aligned, 96-bit scalar loads

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

I'm not much of an expert here but this looks reasonable to me.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1114
+  unsigned EltSize = EltTy.getSizeInBits();
+  assert((TotalSize - FirstSize) % EltSize == 0 && (FirstSize % EltSize) == 0);
+
----------------
All you need is `assert(FirstSize % EltSize == 0)`, no parens around `%`. The other part of the condition is obviously implied.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:1127-1128
+
+  LLT EltTy = Ty.getElementType();
+  return LLT::vector(128 / EltTy.getSizeInBits(), EltTy);
+}
----------------
Perhaps assert that EltTy is a power of 2 size (or equivalently that the divsion `128 / EltTy.getSizeInBits()` is exact) to guard against being called with weird types like v1s96 or v2s48?


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

https://reviews.llvm.org/D81524