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

[Jay Foad via Phabricator via llvm-commits]

foad marked an inline comment as done.
foad 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;
----------------
arsenm wrote:
> foad wrote:
> > arsenm wrote:
> > > 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)
> > But in that case, you should still avoid making drastic changes to vaddr in case it ends up accidentally pointing *into* one of the apertures, when you wanted a global access. E.g. if you're accessing a global that happens to be just past the end of the private or local aperture.
> I thought Nicolai mentioned this might not be possible?
> 
> I guess I don't understand why the aperture is so complicated and not just a couple of flags in the high, unused bits
I don't remember hearing that it wasn't possible. @nhaehnle ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83394