[AArch64] Fix arm neon vstx lane memVT size

[hstk30]

Summary

StN lane memory size set too big lead to alias analysis goes wrong.
https://github.com/llvm/llvm-project/issues/64696

[dmgreen]

Looks good. Thanks for putting this together. It might be worth checking the tests in the pre-commit, to make sure they are passing.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

13911–13915

This can probably be something like

auto *StructTy = cast<StructType>(RetTy);
unsigned VecNum = StructTy->getNumElements();
Type *VecTy = StructTy->getElementType(0);

The cast will then assert that the RetTy is a StructType.

llvm/test/CodeGen/AArch64/arm64-neon-st-lane-aa.ll

2

You might not need -O3, or --check-prefix=CHECK. The check lines could also be generated with update_llc_test_checks. It's a good way to generate consistent tests if it does what you need.

Also a quick comment explaining that the order of the st2 should be before the ldrb to the same address might be useful in the long run.

[hstk30]

This test case LLVM.CodeGen/AArch64/GlobalISel::arm64-irtranslator.ll fail, cause the :: intrinsic info not equal s384 != <12 * s32> .
Seem <12 * s32> is more direct? I don't know where it'll affect. @dmgreen

[dmgreen]

I believe that should be fine, so long as the total size is the same.

[hstk30]

refactor, pass test case

[hstk30]

pass test case

[hstk30]

Passed the fail test case. Have anything to be improved? @dmgreen

[hstk30]

Hi Eli, take a look. Have anything else to be improved? @efriedma

[efriedma]

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

13903

Please also fix the computation for ld2r etc.

[hstk30]

This comment was removed by hstk30.

[hstk30]

So, I also want to add some test cases for LD like file multi-vector-store-size.ll,
but I can't construct simple cases.

Can you give me an example for ld, LD1Twov4s ...

[efriedma]

If you're just having trouble figuring out how to call the intrinsics from IR, there should be a variety of examples in LLVM and clang regression tests. See, for example, llvm/test/CodeGen/AArch64/arm64-ld1.ll .

Otherwise, I'm not sure what you're asking for.

[hstk30]

fix ld2r etc, and test case

[efriedma]

LGTM

[hstk30]

fix typo error

[hstk30]

pass test case

[dmgreen]

LGTM. Do you have commit access? If not we are happy to commit it for you, we would just need an "name <[e-mail]>" for the git attribution. Thanks.

[hstk30]

"hstk30 <[e-mail]>", thanks.

[hstk30]

refactor

[hstk30]

Hi, @dmgreen passed the test cases, please commit it "hstk30 <[e-mail]>", thanks

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

 //===-- AArch64ISelLowering.cpp - AArch64 DAG Lowering Implementation  ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
bool AArch64TargetLowering::getTgtMemIntrinsic(IntrinsicInfo &Info,
   case Intrinsic::aarch64_neon_ld1x2:
   case Intrinsic::aarch64_neon_ld1x3:
   case Intrinsic::aarch64_neon_ld1x4:
   case Intrinsic::aarch64_neon_ld2lane:
   case Intrinsic::aarch64_neon_ld3lane:
   case Intrinsic::aarch64_neon_ld4lane:
   case Intrinsic::aarch64_neon_ld2r:
   case Intrinsic::aarch64_neon_ld3r:
   case Intrinsic::aarch64_neon_ld4r: {
     Info.opc = ISD::INTRINSIC_W_CHAIN;
-    // Conservatively set memVT to the entire set of vectors loaded.
-    uint64_t NumElts = DL.getTypeSizeInBits(I.getType()) / 64;
-    Info.memVT = EVT::getVectorVT(I.getType()->getContext(), MVT::i64, NumElts);
+    unsigned VNumPerVec = 0;
+    // ldx return struct with the same vec type
+    Type *RetTy = I.getType();
+    auto *StructTy = cast<StructType>(RetTy);
+    unsigned VecNum = StructTy->getNumElements();
+    Type *VecTy = StructTy->getElementType(0);
+    MVT VecVT = MVT::getVT(VecTy);
+    MVT EleVT = VecVT.getVectorElementType();
+
+    if (Intrinsic == Intrinsic::aarch64_neon_ld2lane ||
+        Intrinsic == Intrinsic::aarch64_neon_ld3lane ||
+        Intrinsic == Intrinsic::aarch64_neon_ld4lane)
+      VNumPerVec = 1;
+    else
+      VNumPerVec = VecVT.getVectorNumElements();
+
+    Info.memVT =
+        EVT::getVectorVT(I.getType()->getContext(), EleVT, VecNum * VNumPerVec);
     Info.ptrVal = I.getArgOperand(I.arg_size() - 1);
     Info.offset = 0;
     Info.align.reset();
     // volatile loads with NEON intrinsics not supported
     Info.flags = MachineMemOperand::MOLoad;
     return true;
   }
   case Intrinsic::aarch64_neon_st2:
   case Intrinsic::aarch64_neon_st3:
   case Intrinsic::aarch64_neon_st4:
   case Intrinsic::aarch64_neon_st1x2:
   case Intrinsic::aarch64_neon_st1x3:
   case Intrinsic::aarch64_neon_st1x4:
   case Intrinsic::aarch64_neon_st2lane:
   case Intrinsic::aarch64_neon_st3lane:
   case Intrinsic::aarch64_neon_st4lane: {
     Info.opc = ISD::INTRINSIC_VOID;
-    // Conservatively set memVT to the entire set of vectors stored.
-    unsigned NumElts = 0;
+    unsigned VecNum = 0;
+    unsigned VNumPerVec = 0;
+    // all the vector type is same
+    Type *VecTy = I.getArgOperand(0)->getType();
+    MVT VecVT = MVT::getVT(VecTy);
+    MVT EleVT = VecVT.getVectorElementType();
+
     for (const Value *Arg : I.args()) {
       Type *ArgTy = Arg->getType();
       if (!ArgTy->isVectorTy())
         break;
-      NumElts += DL.getTypeSizeInBits(ArgTy) / 64;
+      VecNum += 1;
     }
-    Info.memVT = EVT::getVectorVT(I.getType()->getContext(), MVT::i64, NumElts);
+
+    if (Intrinsic == Intrinsic::aarch64_neon_st2lane ||
+        Intrinsic == Intrinsic::aarch64_neon_st3lane ||
+        Intrinsic == Intrinsic::aarch64_neon_st4lane)
+      VNumPerVec = 1;
+    else
+      VNumPerVec = VecVT.getVectorNumElements();
+
+    Info.memVT =
+        EVT::getVectorVT(I.getType()->getContext(), EleVT, VecNum * VNumPerVec);
     Info.ptrVal = I.getArgOperand(I.arg_size() - 1);
     Info.offset = 0;
     Info.align.reset();
     // volatile stores with NEON intrinsics not supported
     Info.flags = MachineMemOperand::MOStore;
     return true;
   }
   case Intrinsic::aarch64_ldaxr:

llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

 define float @test_nearbyint_f32(float %x) {
   ; CHECK-LABEL: name:            test_nearbyint_f32
   ; CHECK: %{{[0-9]+}}:_(s32) = G_FNEARBYINT %{{[0-9]+}}
   %y = call float @llvm.nearbyint.f32(float %x)
   ret float %y
 }
 
 ; CHECK-LABEL: name: test_llvm.aarch64.neon.ld3.v4i32.p0i32
-; CHECK: %1:_(<4 x s32>), %2:_(<4 x s32>), %3:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aarch64.neon.ld3), %0(p0) :: (load (s384) from %ir.ptr, align 64)
+; CHECK: %1:_(<4 x s32>), %2:_(<4 x s32>), %3:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.aarch64.neon.ld3), %0(p0) :: (load (<12 x s32>) from %ir.ptr, align 64)
 define void @test_llvm.aarch64.neon.ld3.v4i32.p0i32(ptr %ptr) {
   %arst = call { <4 x i32>, <4 x i32>, <4 x i32> } @llvm.aarch64.neon.ld3.v4i32.p0(ptr %ptr)
   ret void
 }
 
 declare { <4 x i32>, <4 x i32>, <4 x i32> } @llvm.aarch64.neon.ld3.v4i32.p0(ptr) #3
 
 define void @test_i1_arg_zext(ptr %f) {

llvm/test/CodeGen/AArch64/arm64-neon-st-lane-aa.ll

+; RUN: llc < %s -mtriple=arm64-none-linux-gnu -mattr=+neon -O2 | FileCheck %s 
+
+; st2 must before two ldrb.
+; The situation that put one ldrb before st2 because of the conservative memVT set for st2lane,
+; which lead to basic-aa goes wrong.
+ 
+define dso_local i32 @test_vst2_lane_u8([2 x <8 x i8>] %vectors.coerce) local_unnamed_addr {
+; CHECK-LABEL:   test_vst2_lane_u8:
+; CHECK:         st2 { v0.b, v1.b }[6], [x8]
+; CHECK-NEXT:    umov w8, v1.b[6]
+; CHECK-NEXT:    ldrb w10, [sp, #12]
+; CHECK-NEXT:    ldrb w11, [sp, #13]
+entry:
+  %temp = alloca [2 x i8], align 4
+  %vectors.coerce.fca.0.extract = extractvalue [2 x <8 x i8>] %vectors.coerce, 0
+  %vectors.coerce.fca.1.extract = extractvalue [2 x <8 x i8>] %vectors.coerce, 1
+  call void @llvm.lifetime.start.p0(i64 2, ptr nonnull %temp) #4
+  call void @llvm.aarch64.neon.st2lane.v8i8.p0(<8 x i8> %vectors.coerce.fca.0.extract, <8 x i8> %vectors.coerce.fca.1.extract, i64 6, ptr nonnull %temp)
+  %0 = load i8, ptr %temp, align 4
+  %vget_lane = extractelement <8 x i8> %vectors.coerce.fca.0.extract, i64 6
+  %cmp8.not = icmp ne i8 %0, %vget_lane
+  %arrayidx3.1 = getelementptr inbounds [2 x i8], ptr %temp, i64 0, i64 1
+  %1 = load i8, ptr %arrayidx3.1, align 1
+  %vget_lane.1 = extractelement <8 x i8> %vectors.coerce.fca.1.extract, i64 6
+  %cmp8.not.1 = icmp ne i8 %1, %vget_lane.1
+  %or.cond = select i1 %cmp8.not, i1 true, i1 %cmp8.not.1
+  %cmp.lcssa = zext i1 %or.cond to i32
+  call void @llvm.lifetime.end.p0(i64 2, ptr nonnull %temp) #4
+  ret i32 %cmp.lcssa
+}
+ 
+declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #2
+declare void @llvm.aarch64.neon.st2lane.v8i8.p0(<8 x i8>, <8 x i8>, i64, ptr nocapture) #2
+declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture) #2

llvm/test/CodeGen/AArch64/multi-vector-store-size.ll

define void @addstx(ptr %res, ptr %a,  ptr %b, ptr %c, ptr %d) {
   %cl = load <4 x float>, ptr %c
   %dl = load <4 x float>, ptr %d
 
   %ar = fadd <4 x float> %al, %bl
   %br = fadd <4 x float> %bl, %cl
   %cr = fadd <4 x float> %cl, %dl
   %dr = fadd <4 x float> %dl, %al
 
-; The sizes below are conservative.  AArch64TargetLowering
-; conservatively assumes the entire vector is stored.
   tail call void @llvm.aarch64.neon.st2.v4f32.p0(<4 x float> %ar, <4 x float> %br, ptr %res)
 ; CHECK: ST2Twov4s {{.*}} :: (store (s256) {{.*}})
   tail call void @llvm.aarch64.neon.st3.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, ptr %res)
 ; CHECK: ST3Threev4s {{.*}} :: (store (s384) {{.*}})
   tail call void @llvm.aarch64.neon.st4.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, <4 x float> %dr, ptr %res)
 ; CHECK: ST4Fourv4s {{.*}} :: (store (s512) {{.*}})
 
   ret void
 }
 
 define void @addst1x(ptr %res, ptr %a,  ptr %b, ptr %c, ptr %d) {
   %al = load <4 x float>, ptr %a
   %bl = load <4 x float>, ptr %b
   %cl = load <4 x float>, ptr %c
   %dl = load <4 x float>, ptr %d
 
   %ar = fadd <4 x float> %al, %bl
   %br = fadd <4 x float> %bl, %cl
   %cr = fadd <4 x float> %cl, %dl
   %dr = fadd <4 x float> %dl, %al
 
-; The sizes below are conservative.  AArch64TargetLowering
-; conservatively assumes the entire vector is stored.
   tail call void @llvm.aarch64.neon.st1x2.v4f32.p0(<4 x float> %ar, <4 x float> %br, ptr %res)
 ; CHECK: ST1Twov4s {{.*}} :: (store (s256) {{.*}})
   tail call void @llvm.aarch64.neon.st1x3.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, ptr %res)
 ; CHECK: ST1Threev4s {{.*}} :: (store (s384) {{.*}})
   tail call void @llvm.aarch64.neon.st1x4.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, <4 x float> %dr, ptr %res)
 ; CHECK: ST1Fourv4s {{.*}} :: (store (s512) {{.*}})
 
   ret void
 }
 
 define void @addstxlane(ptr %res, ptr %a,  ptr %b, ptr %c, ptr %d) {
   %al = load <4 x float>, ptr %a
   %bl = load <4 x float>, ptr %b
   %cl = load <4 x float>, ptr %c
   %dl = load <4 x float>, ptr %d
 
   %ar = fadd <4 x float> %al, %bl
   %br = fadd <4 x float> %bl, %cl
   %cr = fadd <4 x float> %cl, %dl
   %dr = fadd <4 x float> %dl, %al
 
-; The sizes below are conservative.  AArch64TargetLowering
-; conservatively assumes the entire vector is stored.
   tail call void @llvm.aarch64.neon.st2lane.v4f32.p0(<4 x float> %ar, <4 x float> %br, i64 1, ptr %res)
-; CHECK: ST2i32 {{.*}} :: (store (s256) {{.*}})
+; CHECK: ST2i32 {{.*}} :: (store (s64) {{.*}})
   tail call void @llvm.aarch64.neon.st3lane.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, i64 1, ptr %res)
-; CHECK: ST3i32 {{.*}} :: (store (s384) {{.*}})
+; CHECK: ST3i32 {{.*}} :: (store (s96) {{.*}})
   tail call void @llvm.aarch64.neon.st4lane.v4f32.p0(<4 x float> %ar, <4 x float> %br, <4 x float> %cr, <4 x float> %dr, i64 1, ptr %res)
-; CHECK: ST4i32 {{.*}} :: (store (s512) {{.*}})
+; CHECK: ST4i32 {{.*}} :: (store (s128) {{.*}})
 
   ret void
 }