[LV] Avoid scalable vectorization for loops containing alloca

[kmclaughlin]

Summary

This patch returns an Invalid cost from getInstructionCost() for alloca
instructions if the VF is scalable, as otherwise loops which contain
these instructions will crash when attempting to scalarize the alloca.

[sdesmalen]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7836

Can you add a comment describing why we don't want to vectorize this for scalable vectors?

llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll

29

Can you force the loop to be vectorized with a scalable VF using a loop-hint? With the current RUN line and without hints, the loop may make the decision for other reasons (like considering a fixed-VF cheaper, rather than outright ignoring a scalable VF that was otherwise forced, but had an invalid cost).

It may also be worth rebasing your patch on D105806 and adding a CHECK line for one of the remarks (although the remarks may still change depending on review feedback)

[david-arm]

llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll

32

Do we need this alloca and the call to @foo at the bottom? I wonder if it may be simpler to pass a i32** %vla pointer to the function? I think the main thing we care about is the alloca in the loop I think?

33

nit: I think we can simplify the test here by just branching directly, i.e.

br label %for.body

without the icmp.

[kmclaughlin]

Changes to scalable-alloca.ll:

• Added a loop hint to force vectorization with a scalable VF
• Rebased the patch and added CHECK lines for Invalid costs found in the loop (from D105806)
• Passed i32** %vla to the @alloca function

[CarolineConcatto]

llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll

1

Makes sense to use the python file to create the check lines here?

[sdesmalen]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7838

nit: this line is redundant, because the code says as much.

llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll

1

@CarolineConcatto I don't think that is necessary for this specific patch because this test is not guarding the fixed-width vectorization capabilities of the LV, nor the AArch64 CostModel.

4

Can you also add: CHECK-REMARKS: UserVF ignored because of invalid costs.

9

I'd suggest to remove these CHECK lines, because the fact that this loop vectorizes with fixed-width vectors is down to the cost-model, which means we may have to regenerate the check lines at some point, even though they're not needed for what it is that you're trying to test.

[kmclaughlin]

• Removed unnecessary CHECK lines from scalable-alloca.ll
• Added a check for the "UserVF ignored because of invalid costs" remark

[sdesmalen]

LGTM, thanks.

Just FYI, you'll want to wait with landing it until I've relanded D105806.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

case Instruction::Call: {
     if (getVectorIntrinsicIDForCall(CI, TLI)) {
       InstructionCost IntrinsicCost = getVectorIntrinsicCost(CI, VF);
       return std::min(CallCost, IntrinsicCost);
     }
     return CallCost;
   }
   case Instruction::ExtractValue:
     return TTI.getInstructionCost(I, TTI::TCK_RecipThroughput);
+  case Instruction::Alloca:
+    if (VF.isScalable())
+      return InstructionCost::getInvalid();
+    LLVM_FALLTHROUGH;
   default:
     // This opcode is unknown. Assume that it is the same as 'mul'.
     return TTI.getArithmeticInstrCost(Instruction::Mul, VectorTy, CostKind);
   } // end of switch.
 }
 
 char LoopVectorize::ID = 0;
 

llvm/test/Transforms/LoopVectorize/AArch64/scalable-alloca.ll

+; RUN: opt -S -loop-vectorize -mattr=+sve -mtriple aarch64-unknown-linux-gnu -scalable-vectorization=on < %s | FileCheck %s
+
+define void @alloca(i64 %N) {
+; CHECK-LABEL: @alloca(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[VLA:%.*]] = alloca i32*, i64 [[N:%.*]], align 8
+; CHECK:       vector.body:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i64 [ 0, %vector.ph ], [ [[INDEX_NEXT:%.*]], %vector.body ]
+; CHECK-NEXT:    [[INDUCTION:%.*]] = add i64 [[INDEX]], 0
+; CHECK-NEXT:    [[INDUCTION1:%.*]] = add i64 [[INDEX]], 1
+; CHECK-NEXT:    [[TMP0:%.*]] = alloca i32, align 16
+; CHECK-NEXT:    [[TMP1:%.*]] = alloca i32, align 16
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr inbounds i32*, i32** [[VLA]], i64 [[INDUCTION]]
+; CHECK-NEXT:    [[TMP3:%.*]] = getelementptr inbounds i32*, i32** [[VLA]], i64 [[INDUCTION1]]
+; CHECK-NEXT:    store i32* [[TMP0]], i32** [[TMP2]], align 8
+; CHECK-NEXT:    store i32* [[TMP1]], i32** [[TMP3]], align 8
+; CHECK:       scalar.ph:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i64 [ {{.*}}, %middle.block ], [ 0, %for.body.preheader ]
+; CHECK-NEXT:    br label %for.body
+; CHECK:       for.body:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ [[IV_NEXT:%.*]], %for.body ], [ [[BC_RESUME_VAL]], %scalar.ph ]
+; CHECK-NEXT:    [[ALLOCA:%.*]] = alloca i32, align 16
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32*, i32** [[VLA]], i64 [[IV]]
+; CHECK-NEXT:    store i32* [[ALLOCA]], i32** [[ARRAYIDX]], align 8
+; CHECK:       for.end:
+; CHECK-NEXT:    call void @foo(i32** nonnull [[VLA]])
+; CHECK-NEXT:    ret void
+
+; CHECK-NOT: <vscale x
+
+entry:
+  %vla = alloca i32*, i64 %N, align 8
+  %cmp5 = icmp sgt i64 %N, 0
+  br i1 %cmp5, label %for.body, label %for.end
+
+for.body:
+  %iv = phi i64 [ %iv.next, %for.body ], [ 0, %entry ]
+  %alloca = alloca i32, align 16
+  %arrayidx = getelementptr inbounds i32*, i32** %vla, i64 %iv
+  store i32* %alloca, i32** %arrayidx, align 8
+  %iv.next = add nuw nsw i64 %iv, 1
+  %exitcond.not = icmp eq i64 %iv.next, %N
+  br i1 %exitcond.not, label %for.end, label %for.body
+
+for.end:
+  call void @foo(i32** nonnull %vla)
+  ret void
+}
+
+declare void @foo(i32**)