[PATCH] D68200: [AMDGPU] Extend buffer intrinsics with swizzling

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

In D68200#1688092 <https://reviews.llvm.org/D68200#1688092>, @arsenm wrote:

> I thought this was a property of the resource descriptor? Why do you need to add it to the intrinsic?


Yes, but it is illegal to merge load/store instructions that access swizzled buffers. So far our front-end has been using tbuffer stores in such cases to work around that issue, as tbuffer loads/stores are not being merged in si-load-store-opt. However, since it is often profitable to merge tbuffer loads/stores we want to implement tbuffer load/store merging and selectively prevent merging for instructions that are tagged as operating on a swizzled buffer.



================
Comment at: include/llvm/IR/IntrinsicsAMDGPU.td:903
    llvm_i32_ty],      // cachepolicy(imm; bit 0 = glc, bit 1 = slc, bit 2 = dlc on gfx10+)
+                      // swizzled buffer(imm; bit 3 = swz)
   [IntrReadMem, ImmArg<3>], "", [SDNPMemOperand]>,
----------------
arsenm wrote:
> arsenm wrote:
> > Need to use immarg for all of these
> Actually this isn't adding a new argument, but also doesn't have anything to do with the cachepolicy
Yes, the new bit is not really about the cache policy. I should probably rename "cachepolicy" to something more aptly named, or at least rewrite the comment here to make it clear.

The idea behind re-using the cachepolicy operand is to avoid the need to create yet another generation of intrinsics. Also, it makes it backward compatible, the front-ends can enable the "swizzled" bit at their own time. 


================
Comment at: lib/Target/AMDGPU/SILoadStoreOptimizer.cpp:522
 
+  /* Do not merge VMEM buffer instructions with "swizzled" bit set. */
+  int swizzled = AMDGPU::getNamedOperandIdx(CI.I->getOpcode(), AMDGPU::OpName::swz);
----------------
arsenm wrote:
> Single line comment
Ah yes, sorry I keep making this error. I will use the C++-style comment and also rename "swizzled" to "Swizzled".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68200