[VPlan] Return invalid cost if any skeleton block has invalid costs. #151940

fhahn · 2025-08-04T10:57:06Z

We need to reject plans that contain recipes with invalid costs. LICM can move recipes with invalid costs out of the loop region, which then get missed by the main cost computation.

Extend the logic to check recipes for invalid cost currently only covering the middle block to include all skeleton blocks.

Fixes #144358
Fixes #151664

We need to reject plans that contain recipes with invalid costs. LICM can move recipes with invalid costs out of the loop region, which then get missed by the main cost computation. Extend the logic to check recipes for invalid cost currently only covering the middle block to include all skeleton blocks. Fixes llvm#144358 Fixes llvm#151664

llvmbot · 2025-08-04T10:57:50Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

Changes

We need to reject plans that contain recipes with invalid costs. LICM can move recipes with invalid costs out of the loop region, which then get missed by the main cost computation.

Extend the logic to check recipes for invalid cost currently only covering the middle block to include all skeleton blocks.

Fixes #144358
Fixes #151664

Full diff: https://github.com/llvm/llvm-project/pull/151940.diff

2 Files Affected:

(modified) llvm/lib/Transforms/Vectorize/VPlan.cpp (+7-3)
(modified) llvm/test/Transforms/LoopVectorize/AArch64/pr151664-cost-hoisted-vector-scalable.ll (+11-30)

diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 2138b4154d2c2..9cc62247ee553 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1072,9 +1072,13 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
   // blocks, like the preheader or middle blocks.
   InstructionCost Cost = getVectorLoopRegion()->cost(VF, Ctx);
 
-  // If any instructions in the middle block are invalid return invalid.
-  // TODO: Remove once no VPlans with VF == vscale x 1 and first-order recurrences are created.
-  if (!getMiddleBlock()->cost(VF, Ctx).isValid())
+  // If any instructions in the skeleton outside loop regions are invalid return
+  // invalid.
+  if (any_of(VPBlockUtils::blocksOnly<VPBasicBlock>(
+                 vp_depth_first_shallow(getEntry())),
+             [&VF, &Ctx](VPBasicBlock *VPBB) {
+               return !VPBB->cost(VF, Ctx).isValid();
+             }))
     return InstructionCost::getInvalid();
 
   return Cost;
diff --git a/llvm/test/Transforms/LoopVectorize/AArch64/pr151664-cost-hoisted-vector-scalable.ll b/llvm/test/Transforms/LoopVectorize/AArch64/pr151664-cost-hoisted-vector-scalable.ll
index 8495deea31e8b..b4df63dd08fd6 100644
--- a/llvm/test/Transforms/LoopVectorize/AArch64/pr151664-cost-hoisted-vector-scalable.ll
+++ b/llvm/test/Transforms/LoopVectorize/AArch64/pr151664-cost-hoisted-vector-scalable.ll
@@ -1,47 +1,28 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals none --filter-out-after "^scalar.ph" --version 5
-; REQUIRES: asserts
-; RUN: opt -passes=loop-vectorize -mtriple=aarch64 -mattr=+sve -S \
-; RUN:   -debug-only=loop-vectorize %s 2>&1 | FileCheck %s
+; RUN: opt -passes=loop-vectorize -mtriple=aarch64 -mattr=+sve -S %s | FileCheck %s
 
-; FIXME: Hoisted vector code should be costed with scalable cost.
-; In this example, `<vscale x 4 x float> @llvm.minimumnum` has an invalid cost,
-; and hence should not be produced by LoopVectorize.
-
-; CHECK: LV: Found an estimated cost of Invalid for VF vscale x 4 For instruction:   %res = tail call float @llvm.minimumnum.f32(float %arg, float 0.000000e+00)
 define void @cost_hoisted_vector_code(ptr %p, float %arg) {
 ; CHECK-LABEL: define void @cost_hoisted_vector_code(
 ; CHECK-SAME: ptr [[P:%.*]], float [[ARG:%.*]]) #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  [[ENTRY:.*:]]
-; CHECK-NEXT:    [[TMP0:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:    [[TMP1:%.*]] = mul nuw i64 [[TMP0]], 8
 ; CHECK-NEXT:    br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
 ; CHECK:       [[VECTOR_PH]]:
-; CHECK-NEXT:    [[TMP2:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:    [[TMP3:%.*]] = mul nuw i64 [[TMP2]], 8
-; CHECK-NEXT:    [[N_MOD_VF:%.*]] = urem i64 -1, [[TMP3]]
-; CHECK-NEXT:    [[N_VEC:%.*]] = sub i64 -1, [[N_MOD_VF]]
-; CHECK-NEXT:    [[TMP4:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:    [[TMP5:%.*]] = mul nuw i64 [[TMP4]], 8
-; CHECK-NEXT:    [[BROADCAST_SPLATINSERT:%.*]] = insertelement <vscale x 4 x float> poison, float [[ARG]], i64 0
-; CHECK-NEXT:    [[BROADCAST_SPLAT:%.*]] = shufflevector <vscale x 4 x float> [[BROADCAST_SPLATINSERT]], <vscale x 4 x float> poison, <vscale x 4 x i32> zeroinitializer
-; CHECK-NEXT:    [[TMP6:%.*]] = add i64 1, [[N_VEC]]
-; CHECK-NEXT:    [[TMP7:%.*]] = call <vscale x 4 x float> @llvm.minimumnum.nxv4f32(<vscale x 4 x float> [[BROADCAST_SPLAT]], <vscale x 4 x float> zeroinitializer)
+; CHECK-NEXT:    [[BROADCAST_SPLATINSERT:%.*]] = insertelement <4 x float> poison, float [[ARG]], i64 0
+; CHECK-NEXT:    [[BROADCAST_SPLAT:%.*]] = shufflevector <4 x float> [[BROADCAST_SPLATINSERT]], <4 x float> poison, <4 x i32> zeroinitializer
+; CHECK-NEXT:    [[TMP0:%.*]] = call <4 x float> @llvm.minimumnum.v4f32(<4 x float> [[BROADCAST_SPLAT]], <4 x float> zeroinitializer)
 ; CHECK-NEXT:    br label %[[VECTOR_BODY:.*]]
 ; CHECK:       [[VECTOR_BODY]]:
 ; CHECK-NEXT:    [[INDEX1:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
 ; CHECK-NEXT:    [[INDEX:%.*]] = add i64 1, [[INDEX1]]
 ; CHECK-NEXT:    [[TMP8:%.*]] = getelementptr float, ptr [[P]], i64 [[INDEX]]
-; CHECK-NEXT:    [[TMP9:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:    [[TMP10:%.*]] = mul nuw i64 [[TMP9]], 4
-; CHECK-NEXT:    [[TMP11:%.*]] = getelementptr float, ptr [[TMP8]], i64 [[TMP10]]
-; CHECK-NEXT:    store <vscale x 4 x float> [[TMP7]], ptr [[TMP8]], align 4
-; CHECK-NEXT:    store <vscale x 4 x float> [[TMP7]], ptr [[TMP11]], align 4
-; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i64 [[INDEX1]], [[TMP5]]
-; CHECK-NEXT:    [[TMP12:%.*]] = icmp eq i64 [[INDEX_NEXT]], [[N_VEC]]
-; CHECK-NEXT:    br i1 [[TMP12]], label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr float, ptr [[TMP8]], i32 4
+; CHECK-NEXT:    store <4 x float> [[TMP0]], ptr [[TMP8]], align 4
+; CHECK-NEXT:    store <4 x float> [[TMP0]], ptr [[TMP2]], align 4
+; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i64 [[INDEX1]], 8
+; CHECK-NEXT:    [[TMP3:%.*]] = icmp eq i64 [[INDEX_NEXT]], -8
+; CHECK-NEXT:    br i1 [[TMP3]], label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
 ; CHECK:       [[MIDDLE_BLOCK]]:
-; CHECK-NEXT:    [[CMP_N:%.*]] = icmp eq i64 -1, [[N_VEC]]
-; CHECK-NEXT:    br i1 [[CMP_N]], [[EXIT:label %.*]], label %[[SCALAR_PH]]
+; CHECK-NEXT:    br label %[[SCALAR_PH]]
 ; CHECK:       [[SCALAR_PH]]:
 ;
 entry:

artagnon

Thanks a lot for fixing this!

artagnon · 2025-08-04T11:00:40Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

+  // invalid.
+  if (any_of(VPBlockUtils::blocksOnly<VPBasicBlock>(
+                 vp_depth_first_shallow(getEntry())),
+             [&VF, &Ctx](VPBasicBlock *VPBB) {


Suggested change

[&VF, &Ctx](VPBasicBlock *VPBB) {

[&VF, &Ctx](const VPBasicBlock *VPBB) {

Nit. I hope cost() is marked cost?

ayalz

Couple of post-approval nits, somewhat independent.

ayalz · 2025-08-04T18:40:57Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

@@ -1072,9 +1072,13 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
  // blocks, like the preheader or middle blocks.


Comment deserves updating?

Updated to clarify that other blocks are checked for recipes with invalid costs, thanks.

ayalz · 2025-08-04T18:42:21Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

@@ -1072,9 +1072,13 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
  // blocks, like the preheader or middle blocks.
  InstructionCost Cost = getVectorLoopRegion()->cost(VF, Ctx);


Better bail out first if invalid?

Yep, added a check to the if and updated the comment, thanks!

…eton-for-invalid

…lid costs. (#151940) We need to reject plans that contain recipes with invalid costs. LICM can move recipes with invalid costs out of the loop region, which then get missed by the main cost computation. Extend the logic to check recipes for invalid cost currently only covering the middle block to include all skeleton blocks. Fixes llvm/llvm-project#144358 Fixes llvm/llvm-project#151664 PR: llvm/llvm-project#151940

ayalz · 2025-08-07T10:51:32Z

llvm/lib/Transforms/Vectorize/VPlan.cpp

+  if (!Cost.isValid() || any_of(VPBlockUtils::blocksOnly<VPBasicBlock>(
+                                    vp_depth_first_shallow(getEntry())),
+                                [&VF, &Ctx](VPBasicBlock *VPBB) {
+                                  return !VPBB->cost(VF, Ctx).isValid();
+                                }))


Worth checking the any_of() part and potentially bailing out before computing the Cost of the vector loop?

fhahn requested review from artagnon, davemgreen, ayalz and aniragil August 4, 2025 10:57

llvmbot added vectorizers llvm:transforms labels Aug 4, 2025

This was referenced Aug 4, 2025

[Aarch64][Codegen] fatal error: Invalid size request on a scalable vector #144358

Closed

[LV] Hoisted vector code is costed without vscale #151664

Closed

artagnon approved these changes Aug 4, 2025

View reviewed changes

ayalz reviewed Aug 4, 2025

View reviewed changes

fhahn added 2 commits August 7, 2025 09:37

Merge remote-tracking branch 'origin/main' into vplan-cost-check-skel…

26f3998

…eton-for-invalid

!fixup clarify comments, add early exit for invalid cost

2760573

fhahn merged commit 95c32bf into llvm:main Aug 7, 2025
8 of 9 checks passed

fhahn deleted the vplan-cost-check-skeleton-for-invalid branch August 7, 2025 09:45

fhahn mentioned this pull request Aug 7, 2025

[VPlan] Materialize vector trip count using VPInstructions. #151925

Merged

ayalz reviewed Aug 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[VPlan] Return invalid cost if any skeleton block has invalid costs. #151940

[VPlan] Return invalid cost if any skeleton block has invalid costs. #151940

Uh oh!

fhahn commented Aug 4, 2025

Uh oh!

llvmbot commented Aug 4, 2025 •

edited

Loading

Uh oh!

artagnon left a comment

Uh oh!

artagnon Aug 4, 2025

Uh oh!

ayalz left a comment

Uh oh!

ayalz Aug 4, 2025

Uh oh!

fhahn Aug 7, 2025

Uh oh!

ayalz Aug 4, 2025

Uh oh!

fhahn Aug 7, 2025

Uh oh!

Uh oh!

ayalz Aug 7, 2025

Uh oh!

Uh oh!

	[&VF, &Ctx](VPBasicBlock *VPBB) {
	[&VF, &Ctx](const VPBasicBlock *VPBB) {

		@@ -1072,9 +1072,13 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
		// blocks, like the preheader or middle blocks.

		@@ -1072,9 +1072,13 @@ InstructionCost VPlan::cost(ElementCount VF, VPCostContext &Ctx) {
		// blocks, like the preheader or middle blocks.
		InstructionCost Cost = getVectorLoopRegion()->cost(VF, Ctx);

[VPlan] Return invalid cost if any skeleton block has invalid costs. #151940

[VPlan] Return invalid cost if any skeleton block has invalid costs. #151940

Uh oh!

Conversation

fhahn commented Aug 4, 2025

Uh oh!

llvmbot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artagnon left a comment

Choose a reason for hiding this comment

Uh oh!

artagnon Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

ayalz left a comment

Choose a reason for hiding this comment

Uh oh!

ayalz Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

ayalz Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ayalz Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Aug 4, 2025 •

edited

Loading