[PATCH] D79641: [AMDGPU] Vectorize alloca thru bitcast

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:310
+    V = I->getOperand(0);
+  }
+  return V;
----------------
arsenm wrote:
> I think this needs to be careful around multiple uses
We just need to get to the actual pointer here, it does not matter if there are multiple uses, bitcasts are not removed anyway. Also note that is not a problem if not all uses are converted, alloca itself stays. The pass does partial vectorization even now.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:435
+        for (User *CastUser : Inst->users()) {
+          if (isAssumeLikeIntrinsic(cast<Instruction>(CastUser)))
+            continue;
----------------
arsenm wrote:
> Needs test with assume intrinsic
It's the same as lifetime instrinsics in the test, but I will add one. Just note that a pointer cannot be passed into assume, it shall be a compare, so it will prevent alloca removal.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:481
       Value *ExtractElement = Builder.CreateExtractElement(VecValue, Index);
+      if (Inst->getType() != VecEltTy)
+        ExtractElement = Builder.CreateBitCast(ExtractElement, Inst->getType());
----------------
arsenm wrote:
> IRBuilder does this check for you so you can omit it
Actually it does not: Assertion `New->getType() == getType() && "replaceAllUses of value with new value of different type!"' failed.


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

https://reviews.llvm.org/D79641