[PATCH] D146737: [AMDGPU] Trim zero components from buffer and image stores

[Mateja Marjanovic via Phabricator via llvm-commits]

matejam added a comment.

Thanks for the review.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:1115
+
+    int DMaskIdx = dyn_cast<ConstantInt>(II.getArgOperand(1)) ? 1 : -1;
+    if (simplifyAMDGCNMemoryIntrinsicDemanded(IC, II, DemandedElts, DMaskIdx, false))
----------------
arsenm wrote:
> foad wrote:
> > I think these intrinsics all have `immarg` on the dmask argument, so it should always be a constant?
> Yes, should be able to just cast<ConstantInt>
Both image and buffer intrinsics have the same body in this switch.
For buffer instructions the first operand is not a constant, that is why I used dyn_cast<ConstantInt>.
I could use isa<ConstantInt>.
I'm not sure how would I check for hasNamedOperand, because II is a call instruction.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:358
 
+static void findDemandedElts(InstCombiner &IC, Value *UseV,
+                             Instruction *I, APInt &DemandedElts) {
----------------
foad wrote:
> This function needs a better name now to explain what it does, and a comment, and should return APInt instead of taking an APInt& argument.
Is the name trimTrailingZerosInVector more explanatory?


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

https://reviews.llvm.org/D146737