[PATCH] D51203: AMDGPU: Handle 32-bit address wraparounds for SMRD opcodes

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1362-1369
+  // A 32-bit (address + offset) can wrap around, while the same expression
+  // might not wrap around in 64 bits. We need to fix this case for tests that
+  // do (Addr - BigConst + BigConst), so assume that a "sufficiently small"
+  // offset never causes a wraparound. The chosen number is a guess. Address
+  // wraparounds are never expected to occur in normal apps.
+  if (Addr32Bit && (uint64_t)ByteOffset > 256 * 1024)
+    return false;
----------------
mareko wrote:
> arsenm wrote:
> > Can we check NUW flags on the add instead?
> Yes, but where would I check for 256 * 1024?
The question is, why the check for 256 * 1024 in the first place? That seems rather fishy. I guess it could be made to work if we define that the 32-bit address space only goes up to 4GB - 256KB, but then that should be added to the address space documentation.

The other issue is that I think some hardware generations have the IMM offset as a *signed* byte offset. At least for gfx9 that's what the docs say. So then the check should depend on the hardware generation.


Repository:
  rL LLVM

https://reviews.llvm.org/D51203