[PATCH] D83394: [AMDGPU] Avoid splitting FLAT offsets in unsafe ways

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp:1705
+          // Use signed division by a power of two to truncate towards 0.
+          int64_t D = 1LL << (NumBits - 1);
+          RemainderOffset = (static_cast<int64_t>(COffsetVal) / D) * D;
----------------
foad wrote:
> arsenm wrote:
> > arsenm wrote:
> > > foad wrote:
> > > > arsenm wrote:
> > > > > This limitation also only needs to be applied if AS == FLAT_ADDRESS
> > > > The only "limitation" is that we don't try to split negative offsets if the immediate offset field is unsigned, but you're saying we can do that if AS != FLAT_ADDRESS? What would that mean - that we're using a FLAT instruction but we know statically which part of the address space it is accessing??
> > > Correct. This is always the case pre-gfx9 which did not have the "global" flat instructions
> > Actually pre-gfx9 also didn't have flat offsets. However gfx10 does have a bug with flat offsets, so I think it would still be correct to model this correctly. The instruction patterns do accept either (and global instructions are only preferred through pattern priority)
> > This limitation also only needs to be applied if AS == FLAT_ADDRESS
> 
> I still don't get this. Surely if we're using a FLAT instruction, even if we know which specific address space the programmer is trying to access, we still have to avoid setting vaddr to an address that might point into the wrong aperture.
My understanding was the aperture only means anything for private or local. If it's a global address, it's neither aperture and behaves as a normal instruction (i.e. there's no aperture for global pointers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83394