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

[Marek Olšák via Phabricator via llvm-commits]

mareko 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;
----------------
nhaehnle wrote:
> 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.
arsenm asked me to check for NUW, which would disable IMM offset for all current users. Do you have a better idea?


Repository:
  rL LLVM

https://reviews.llvm.org/D51203