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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec 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:
> > 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.


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

https://reviews.llvm.org/D124884