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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm 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:
> 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


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

https://reviews.llvm.org/D125279