[PATCH] D113489: [AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR

[David Truby via Phabricator via llvm-commits]

DavidTruby added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:732
+  Type *VecTyPtr = II.getType()->getPointerTo();
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint(&II);
----------------
Nit: I think the default template arguments should just be picked without the `<>`


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:750
+  Value *PtrOp = II.getOperand(1);
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint(&II);
----------------



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:765
+  Type *VecTyPtr = VecOp->getType()->getPointerTo();
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint(&II);
----------------



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:769
+  Value *VecPtr = Builder.CreateBitCast(PtrOp, VecTyPtr);
+  (void)Builder.CreateStore(VecOp, VecPtr);
+  return IC.eraseInstFromFunction(II);
----------------
peterwaller-arm wrote:
> paulwalker-arm wrote:
> > Is this necessary?
> Adding `(void)` was my guidance, which on reflection I think was in error, thanks for pointing it out.
If you wanted to make sure you didn't get unused return value warnings you can use `LLVM_ATTRIBUTE_UNUSED` to be a bit more clear than the void cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113489