[PATCH] D79288: [AMDGPU][MC] Enabled 21-bit signed offsets for SMEM instructions

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp:372-373
+  auto Offset = MO.getImm();
+  if (AMDGPU::isVI(STI)) {
+    Offset &= 0xFFFFF;       // VI only supports 20-bit unsigned offsets.
+  }
----------------
dp wrote:
> arsenm wrote:
> > Do we really need to explicitly clamp this here?
> This is not strictly necessary. However AMD documentation states that unused bits in instruction encoding should be zero. An MC instruction may come directly from codegen, and I’m not sure it does SMEM offset validation. Should I replace this truncation with an assert?
I would assume codegen would catch this in a verifier, and the assembler verifier would prevent this from getting here, so this should probably be an assert


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

https://reviews.llvm.org/D79288