[llvm] LoopVectorize: fix phi cost when it is scalar after vectorization (PR #74456)

[Ramkumar Ramachandra via llvm-commits]

https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/74456

>From bf8281d213ef67f35702b08c7430645075768d1a Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Tue, 19 Dec 2023 11:53:23 +0000
Subject: [PATCH 1/2] LoopVectorize/X86: add test for crash in #72969

---
 .../Transforms/LoopVectorize/X86/pr72969.ll   | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 llvm/test/Transforms/LoopVectorize/X86/pr72969.ll

diff --git a/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll b/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll
new file mode 100644
index 00000000000000..72c8b16becfc3e
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll
@@ -0,0 +1,24 @@
+; RUN: not --crash opt -mtriple=x86_64 -mattr=-avx,-avx2,-avx512f,-sse2,-sse3,+sse -passes=loop-vectorize -S < %s
+
+ at h = global i32 0, align 4
+
+define void @test(ptr %p) {
+entry:
+  br label %for.body
+
+for.body:
+  %idx.ext.merge = phi i64 [ 1, %entry ], [ %idx, %for.body ]
+  %inc.merge = phi i16 [ 1, %entry ], [ %inc, %for.body ]
+  %idx.merge = phi i64 [ 0, %entry ], [ %idx.ext.merge, %for.body ]
+  %add = shl i64 %idx.merge, 1
+  %arrayidx = getelementptr [1 x i32], ptr %p, i64 0, i64 %add
+  %0 = load i32, ptr %arrayidx, align 4
+  %inc = add i16 %inc.merge, 1
+  %idx = zext i16 %inc to i64
+  %gep = getelementptr i32, ptr %p, i64 %idx
+  %cmp = icmp ugt ptr %gep, @h
+  br i1 %cmp, label %exit, label %for.body
+
+exit:
+  ret void
+}

>From 75c4e93484e9d9de33180ce595f5ef371e59f652 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Tue, 19 Dec 2023 12:34:52 +0000
Subject: [PATCH 2/2] LoopVectorize: fix phi cost when it is scalar after
 vectorization

In getInstructionCost, when a phi node is identified as a fixed-order
recurrence, and the vectorization factor indicates that we're
vectorizing, we return the cost of a shuffle. However, if it also
satisfies the condition isScalarAfterVectorization, attempting to get
the shuffle cost will result in a crash from a cast. Fix this.

Fixes #72969.
---
 .../Transforms/Vectorize/LoopVectorize.cpp    |  5 ++++-
 .../Transforms/LoopVectorize/X86/pr72969.ll   | 22 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index f82e161fb846d1..73ae3a62d9aae9 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7053,7 +7053,10 @@ LoopVectorizationCostModel::getInstructionCost(Instruction *I, ElementCount VF,
     auto *Phi = cast<PHINode>(I);
 
     // First-order recurrences are replaced by vector shuffles inside the loop.
-    if (VF.isVector() && Legal->isFixedOrderRecurrence(Phi)) {
+    // However, if the Phi is a scalar after vectorization, it won't be
+    // converted into a shuffle.
+    if (VF.isVector() && Legal->isFixedOrderRecurrence(Phi) &&
+        !isScalarAfterVectorization(Phi, VF)) {
       SmallVector<int> Mask(VF.getKnownMinValue());
       std::iota(Mask.begin(), Mask.end(), VF.getKnownMinValue() - 1);
       return TTI.getShuffleCost(TargetTransformInfo::SK_Splice,
diff --git a/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll b/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll
index 72c8b16becfc3e..4cf0399438e084 100644
--- a/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll
+++ b/llvm/test/Transforms/LoopVectorize/X86/pr72969.ll
@@ -1,8 +1,28 @@
-; RUN: not --crash opt -mtriple=x86_64 -mattr=-avx,-avx2,-avx512f,-sse2,-sse3,+sse -passes=loop-vectorize -S < %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -mtriple=x86_64 -mattr=-avx,-avx2,-avx512f,-sse2,-sse3,+sse -passes=loop-vectorize -S < %s | FileCheck %s
 
 @h = global i32 0, align 4
 
 define void @test(ptr %p) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[IDX_EXT_MERGE:%.*]] = phi i64 [ 1, [[ENTRY:%.*]] ], [ [[IDX:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[INC_MERGE:%.*]] = phi i16 [ 1, [[ENTRY]] ], [ [[INC:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[IDX_MERGE:%.*]] = phi i64 [ 0, [[ENTRY]] ], [ [[IDX_EXT_MERGE]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[ADD:%.*]] = shl i64 [[IDX_MERGE]], 1
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr [1 x i32], ptr [[P]], i64 0, i64 [[ADD]]
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[ARRAYIDX]], align 4
+; CHECK-NEXT:    [[INC]] = add i16 [[INC_MERGE]], 1
+; CHECK-NEXT:    [[IDX]] = zext i16 [[INC]] to i64
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr [[P]], i64 [[IDX]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt ptr [[GEP]], @h
+; CHECK-NEXT:    br i1 [[CMP]], label [[EXIT:%.*]], label [[FOR_BODY]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret void
+;
 entry:
   br label %for.body