[PATCH] D112076: [AArch64][SVE][InstCombine] Combine contiguous gather/scatter to load/store

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:885-888
+    uint64_t AlignN =
+        BasePtr->getPointerAlignment(II.getModule()->getDataLayout()).value();
+    Constant *Align =
+        ConstantInt::get(IntegerType::getInt32Ty(II.getContext()), AlignN);
----------------
With the advice below this can become:
```
Align Alignment = BasePtr->getPointerAlignment(II.getModule()->getDataLayout());
```


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:894-895
+    Ptr = Builder.CreateBitCast(Ptr, VecPtrTy);
+    auto MaskedLoad = Builder.CreateIntrinsic(
+        Intrinsic::masked_load, {Ty, VecPtrTy}, {Ptr, Align, Mask, PassThru});
+    MaskedLoad->takeName(&II);
----------------
Please use `Builder.CreateMaskedLoad` here.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:921-924
+    uint64_t AlignN =
+        BasePtr->getPointerAlignment(II.getModule()->getDataLayout()).value();
+    Constant *Align =
+        ConstantInt::get(IntegerType::getInt32Ty(II.getContext()), AlignN);
----------------
As above.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:931-932
+
+    auto MaskedStore = Builder.CreateIntrinsic(
+        Intrinsic::masked_store, {Ty, VecPtrTy}, {Val, Ptr, Align, Mask});
+    MaskedStore->takeName(&II);
----------------
Please use `Builder.CreateMaskedStore` here.


================
Comment at: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-gatherscatter.ll:10
+
+define <vscale x 2 x double> @test.ld1.gather.index.nxv2f64.stride1(<vscale x 2 x i1> %pred, double* %x, i64 %base) #0 {
+; CHECK-LABEL: @test.ld1.gather.index.nxv2f64.stride1(
----------------
Choose to ignore if you disagree but I don't like seeing `.` in test function names and would prefer `_` to be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112076