[PATCH] D124884: [AMDGPU] Add intrinsic llvm.amdgcn.raw.buffer.load.lds

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:1199-1200
+      case Intrinsic::amdgcn_raw_buffer_load_lds:
+        Info.memVT = MVT::getVT(CI.getArgOperand(1)->getType()->
+                                    getNonOpaquePointerElementType());
+        break;
----------------
rampitec wrote:
> rampitec wrote:
> > rampitec wrote:
> > > arsenm wrote:
> > > > arsenm wrote:
> > > > > rampitec wrote:
> > > > > > arsenm wrote:
> > > > > > > Should use the return / data type?
> > > > > > Changed to return. What do you mean by 'use data type'?
> > > > > You're looking at the pointer element type instead of the return / data type. i.e. we would have i8/i16/i32 return types and you don't need to look at the pointer
> > > > I just noticed there is no return type so this is just introducing a dependency on typed pointers which is a no-go.
> > > > 
> > > > I don't actually see why can't we match these from the buffer intrinsic plus LDS access?
> > > It does not return anything. This instruction does not have vdata. The only way to know the size is by looking at the overloaded LDS base pointer pointee.
> > `LDS address = LDS_base + LDS_offset + inst_offset + (TIDinWave * 4)`
> > 
> > We do not have TIDinWave, certainly not after selection. Even before selection it is extremely problematic.
> > 
> > Why typed pointer is a no-go if that works?
> On top of that MEM_ADDR also depends on the TID. It not the same address as a normal buffer_load would use with the same operands.
Pointee types have been removed from the IR. If this really needs the type it would need to use an attribute on the parameter to carry it which may be new territory 


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

https://reviews.llvm.org/D124884