[AArch64][SVE]Support for SVE instrinsic in AArch64TargetTransformInfo

[duan.db]

Summary

Due to the imperfect support of AArch64TargetTransformInfo for SVE Intrinsic, the loop-reduce pass did not obtain the correct information. It will assign the wrong type to the LSRUser, and the generated assembly appears redundant.

This patch is simple support for AArch64TargetTransformInfo. Please have a look and give some advance.
If it's OK, I will go on with other SVE intrinsic support.

[sdesmalen]

The change itself seems sensible to me. Just left a few comments on the test.

llvm/test/Transforms/LoopStrengthReduce/AArch64/getTgtMemIntrinsic1.ll

1 ↗

(On Diff #389401)

This RUN line doesn't use FileCheck to check the output?

4 ↗

(On Diff #389401)

Can you add some extra CHECK lines, and move the CHECK lines to below define void @example01_sve? It's not clear from the CHECK line that loop-reduce is doing the correct thing.

Some other nits:

• Could you please add a comment with context about what this test is supposed to test?
• Maybe remove the name @example01_sve to something more descriptive?
• Can you rename the file to something more descriptive, e.g. sve-load-store-intrinsics.ll ?

[duan.db]

@sdesmalen Thank you for your review, and so sorry for the delay. I have modified the test case according to your valuable suggestions. This test case checks whether loop-reduce can be assigned the correct type（i.e. Address Type）to the SVE load and store intrinsic. If so, SVE.load will reuse the original induction variable, and compilation is more reasonable.
Any other suggestions about the code?

[duan.db]

Ping. Hi, everyone. Do you agree to commit？

[sdesmalen]

Sorry I missed that you updated the patch.

llvm/test/Transforms/LoopStrengthReduce/AArch64/sve-load-store-intrinsics.ll

7

nit: please also add:

CHECK-LABEL: @sve-load-store-intrinsics

36

Can you duplicate this test, but using ld2 and st2, so that we have at least one test for the structured load/stores?

[paulwalker-arm]

My understand of how MemIntrinsicInfo is used is not great so please forgive some potentially naive questions.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

2070

How literal is the description of this property in TargetTransformInfo.h. Specifically it says:

/// If this is non-null, then analysis/optimization passes can assume that
/// this intrinsic is functionally equivalent to a load/store from this
/// pointer.

However, these are predicated loads and stores and thus they take an extra operand that controls when/if memory is accessed.

Looking at EarlyCSE I can see there exists support for the common masked_load and masked_store intrinsics, where masks are considered (see isNonTargetIntrinsicMatch), but I cannot see such for target intrinsics.

This makes me believe we cannot just populate MemIntrinsicInfo for the SVE intrinsics because the data will be incomplete and potentially cause things like EarlyCSE to do the wrong thing.

2097–2104

Is it safe to not set MatchingId here? My assumption is that means all SVE load/strore intrinsics will have the same Id and thus code like in EarlyCSE.cpp that compares the MatchingId of two instructions may not get whatever guarantee they're trying to obtain.

[paulwalker-arm]

I forgot to mention this before but we now lower Intrinsic::aarch64_sve_ld1 and Intrinsic::aarch64_sve_st1 to either load/store instructions or masked_load/masked_store intrinsics. So perhaps the original need for this patch has gone away?

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp

case Intrinsic::aarch64_neon_ld4:
     break;
   case Intrinsic::aarch64_neon_st2:
   case Intrinsic::aarch64_neon_st3:
   case Intrinsic::aarch64_neon_st4:
     Info.ReadMem = false;
     Info.WriteMem = true;
     Info.PtrVal = Inst->getArgOperand(Inst->arg_size() - 1);
     break;
+  case Intrinsic::aarch64_sve_ld1:
+  case Intrinsic::aarch64_sve_ld2:
+  case Intrinsic::aarch64_sve_ld3:
+  case Intrinsic::aarch64_sve_ld4:
+    Info.ReadMem = true;
+    Info.WriteMem = false;
+    Info.PtrVal = Inst->getArgOperand(1);
+    break;
+  case Intrinsic::aarch64_sve_st1:
+  case Intrinsic::aarch64_sve_st2:
+  case Intrinsic::aarch64_sve_st3:
+  case Intrinsic::aarch64_sve_st4:
+    Info.ReadMem = false;
+    Info.WriteMem = true;
+    Info.PtrVal = Inst->getArgOperand(Inst->arg_size() - 1);
+    break;
   }
 
   switch (Inst->getIntrinsicID()) {
   default:
     return false;
   case Intrinsic::aarch64_neon_ld2:
   case Intrinsic::aarch64_neon_st2:
     Info.MatchingId = VECTOR_LDST_TWO_ELEMENTS;
     break;
   case Intrinsic::aarch64_neon_ld3:
   case Intrinsic::aarch64_neon_st3:
     Info.MatchingId = VECTOR_LDST_THREE_ELEMENTS;
     break;
   case Intrinsic::aarch64_neon_ld4:
   case Intrinsic::aarch64_neon_st4:
     Info.MatchingId = VECTOR_LDST_FOUR_ELEMENTS;
     break;
+  case Intrinsic::aarch64_sve_ld1:
+  case Intrinsic::aarch64_sve_st1:
+  case Intrinsic::aarch64_sve_ld2:
+  case Intrinsic::aarch64_sve_st2:
+  case Intrinsic::aarch64_sve_ld3:
+  case Intrinsic::aarch64_sve_st3:
+  case Intrinsic::aarch64_sve_ld4:
+  case Intrinsic::aarch64_sve_st4:
+    break;
   }
   return true;
 }
 
 /// See if \p I should be considered for address type promotion. We check if \p
 /// I is a sext with right type and used in memory accesses. If it used in a
 /// "complex" getelementptr, we allow it to be promoted without finding other
 /// sext instructions that sign extended the same initial value. A getelementptr

llvm/test/Transforms/LoopStrengthReduce/AArch64/sve-load-store-intrinsics.ll

+; RUN: opt < %s -loop-reduce -S 2>&1 | FileCheck %s
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64-none-linux-gnu"
+
+define void @sve-load-store-intrinsics(i32* noalias nocapture %a, i32* %b, i32* %c, i64 %N) {
+; check that after loop-reduce, sve.load reuses the original induction variable.
+
+; CHECK: %lsr.iv = phi i64 [ %lsr.iv.next, %for.body ], [ 0, %for.body.lr.ph ]
+; CHECK: %scevgep4 = getelementptr i32, i32* %b, i64 %lsr.iv
+; CHECK: %7 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.ld1.nxv4i32(<vscale x 4 x i1> %6, i32* %scevgep4)
+; CHECK: %scevgep3 = getelementptr i32, i32* %c, i64 %lsr.iv
+; CHECK: %8 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.ld1.nxv4i32(<vscale x 4 x i1> %6, i32* %scevgep3)
+; CHECK: %9 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.add.nxv4i32(<vscale x 4 x i1> %6, <vscale x 4 x i32> %7, <vscale x 4 x i32> %8)
+; CHECK: tail call void @llvm.aarch64.sve.st1.nxv4i32(<vscale x 4 x i32> %9, <vscale x 4 x i1> %6, i32* %lsr.iv1)
+
+entry:
+  %0 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.ptrue.nxv4i1(i32 31)
+  %1 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.whilelo.nxv4i1.i64(i64 0, i64 %N)
+  %2 = tail call i1 @llvm.aarch64.sve.ptest.first.nxv4i1(<vscale x 4 x i1> %0, <vscale x 4 x i1> %1)
+  br i1 %2, label %for.body.lr.ph, label %for.cond.cleanup
+
+for.body.lr.ph:                                   ; preds = %entry
+  %3 = tail call i64 @llvm.vscale.i64()
+  %4 = shl nuw nsw i64 %3, 2
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body, %entry
+  ret void
+
+for.body:                                         ; preds = %for.body.lr.ph, %for.body
+  %5 = phi <vscale x 4 x i1> [ %1, %for.body.lr.ph ], [ %9, %for.body ]
+  %i.06 = phi i64 [ 0, %for.body.lr.ph ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %b, i64 %i.06
+  %6 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.ld1.nxv4i32(<vscale x 4 x i1> %5, i32* %arrayidx)
+  %arrayidx1 = getelementptr inbounds i32, i32* %c, i64 %i.06
+  %7 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.ld1.nxv4i32(<vscale x 4 x i1> %5, i32* %arrayidx1)
+  %8 = tail call <vscale x 4 x i32> @llvm.aarch64.sve.add.nxv4i32(<vscale x 4 x i1> %5, <vscale x 4 x i32> %6, <vscale x 4 x i32> %7)
+  %arrayidx2 = getelementptr inbounds i32, i32* %a, i64 %i.06
+  tail call void @llvm.aarch64.sve.st1.nxv4i32(<vscale x 4 x i32> %8, <vscale x 4 x i1> %5, i32* %arrayidx2)
+  %add = add i64 %4, %i.06
+  %9 = tail call <vscale x 4 x i1> @llvm.aarch64.sve.whilelo.nxv4i1.i64(i64 %add, i64 %N)
+  %10 = tail call i1 @llvm.aarch64.sve.ptest.first.nxv4i1(<vscale x 4 x i1> %0, <vscale x 4 x i1> %9)
+  br i1 %10, label %for.body, label %for.cond.cleanup
+}
+
+declare <vscale x 4 x i1> @llvm.aarch64.sve.ptrue.nxv4i1(i32 immarg)
+declare <vscale x 4 x i1> @llvm.aarch64.sve.whilelo.nxv4i1.i64(i64, i64)
+declare <vscale x 4 x i32> @llvm.aarch64.sve.ld1.nxv4i32(<vscale x 4 x i1>, i32*)
+declare <vscale x 4 x i32> @llvm.aarch64.sve.add.nxv4i32(<vscale x 4 x i1>, <vscale x 4 x i32>, <vscale x 4 x i32>)
+declare void @llvm.aarch64.sve.st1.nxv4i32(<vscale x 4 x i32>, <vscale x 4 x i1>, i32* nocapture)
+declare i1 @llvm.aarch64.sve.ptest.first.nxv4i1(<vscale x 4 x i1>, <vscale x 4 x i1>)
+declare i64 @llvm.vscale.i64()
+