[PATCH] D77082: [AMDGPU] Handle SMRD signed offset immediate

Mon Mar 30 15:51:50 PDT 2020

kerbowa added inline comments.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1789
+  // GFX9 amd GFX10 have signed byte immediate offsets.
+  if (Subtarget->hasSMEMImmSignedOffset()) {
+    int64_t ByteOffset = C->getSExtValue();
----------------
arsenm wrote:
> Should this just rely on getSMRDEncodedSignedOffset failing based on the subtarget?
Sure, I'm just following what's done for the LiteralOffset below but that could be improved in the same way.

================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:1270

-Optional<int64_t> getSMRDEncodedOffset(const MCSubtargetInfo &ST,
-                                       int64_t ByteOffset) {
+Optional<int64_t> getSMRDEncodedUnsignedOffset(const MCSubtargetInfo &ST,
+                                               int64_t ByteOffset) {
----------------
rampitec wrote:
> Should we instead pass here Subtarget and distinguish if it has needed mode right in this function?
In that case we would also need to pass something to determine whether it's for S_BUFF since there is no signed offset there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77082