[PATCH] D125279: [AMDGPU] Add llvm.amdgcn.global.load.lds intrinsic

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAMDGPU.td:1789
+   llvm_i32_ty,                        // Data byte size: 1/2/4
+   llvm_i32_ty,                        // imm offset
+   llvm_i32_ty],                       // auxiliary data (imm, cachepolicy (bit 0 = glc/sc0,
----------------
arsenm wrote:
> arsenm wrote:
> > rampitec wrote:
> > > arsenm wrote:
> > > > Does this really need the offset parameter? It's not a buffer intrinsic with the funny different swizzling behavior between scalar and vector, so can't we just match this from the base pointer?
> > > This has 2 separate offsets. VGPR offset is applied to the global memory only, while immediate offset applies to both global and LDS. To have it matching first pointer shall be (g_ptr + const) and the second (l_ptr + const) which we cannot guarantee to preserve.
> > Probably should comment offset behavior here
> You could just not apply the offset if the same constant wasn't used on both pointers. If it's going to be in the intrinsic, there should probably be an optimization to try to put the immediate in the offset field
It should fail to match too often. It is easier to give explicit control to the user. Especially because that is how it is supposed to be used, with the incremental stride by dword.


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

https://reviews.llvm.org/D125279