[PATCH] D150002: [AMDGPU] Fix crash with 160-bit p7's by manually defining getPointerTy

Fri May 12 10:25:52 PDT 2023

arsenm added a comment.

In D150002#4330056 <https://reviews.llvm.org/D150002#4330056>, @krzysz00 wrote:

> I went looking and ran into, for example, TargetLoweringBase::computeRegisterProperties, which assumes the MVT integers all double in size.

Just add it and see it anything breaks

================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:990
+MVT SITargetLowering::getPointerTy(const DataLayout &DL, unsigned AS) const {
+  if (AMDGPUAS::BUFFER_FAT_POINTER == AS && DL.getPointerSizeInBits(AS) > 128)
+    return MVT::v5i32;
----------------
krzysz00 wrote:
> foad wrote:
> > krzysz00 wrote:
> > > foad wrote:
> > > > Nit: why do you need the getPointerSizeInBits test?
> > > Paranoia around making sure this isn't an old data layout or the like. I might even want to make the check more precise.
> > I mean why do you need it at all? Don't we know statically that the size of BUFFER_FAT_POINTER is 160 bits?
> I was concerned that, for example, auto-upgrade might not fire or that you'd otherwise end up in a position where `p7` doesn't have its usual definition and so thought to go ahead and check.
Just drop it, there's no reason to try defending against that here

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150002