Skip to content

[SLP]Make PHICompare comparator follow weak strict ordering requirement #110529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 4, 2024
Merged

[SLP]Make PHICompare comparator follow weak strict ordering requirement #110529

merged 5 commits into from
Oct 4, 2024

Conversation

alexey-bataev
Copy link
Member

@alexey-bataev alexey-bataev commented Sep 30, 2024 •
edited
Loading

No description provided.

@alexey-bataev
[𝘀𝗽𝗿] initial version
8bef364 
Created using spr 1.3.5
@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Alexey Bataev (alexey-bataev)

Changes

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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+67-12)
  • (modified) llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll (+9-9)
  • (modified) llvm/test/Transforms/SLPVectorizer/root-trunc-extract-reuse.ll (+2-2)
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index e45fcb2b5c790c..893fef4095b27c 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -5443,6 +5443,22 @@ BoUpSLP::getReorderingData(const TreeEntry &TE, bool TopToBottom) {
     if (!TE.ReorderIndices.empty())
       return TE.ReorderIndices;
 
+    SmallVector<Instruction *> UserBVHead(TE.Scalars.size());
+    for (auto [I, V] : zip(UserBVHead, TE.Scalars)) {
+      if (!V->hasNUsesOrMore(1))
+        continue;
+      auto *II = dyn_cast<InsertElementInst>(*V->user_begin());
+      if (!II)
+        continue;
+      Instruction *BVHead = nullptr;
+      BasicBlock *BB = II->getParent();
+      while (II && II->hasOneUse() && II->getParent() == BB) {
+        BVHead = II;
+        II = dyn_cast<InsertElementInst>(II->getOperand(0));
+      }
+      I = BVHead;
+    }
+
     auto PHICompare = [&](unsigned I1, unsigned I2) {
       Value *V1 = TE.Scalars[I1];
       Value *V2 = TE.Scalars[I2];
@@ -5454,21 +5470,60 @@ BoUpSLP::getReorderingData(const TreeEntry &TE, bool TopToBottom) {
         return false;
       auto *FirstUserOfPhi1 = cast<Instruction>(*V1->user_begin());
       auto *FirstUserOfPhi2 = cast<Instruction>(*V2->user_begin());
-      if (auto *IE1 = dyn_cast<InsertElementInst>(FirstUserOfPhi1))
-        if (auto *IE2 = dyn_cast<InsertElementInst>(FirstUserOfPhi2)) {
-          if (!areTwoInsertFromSameBuildVector(
-                  IE1, IE2,
-                  [](InsertElementInst *II) { return II->getOperand(0); }))
-            return I1 < I2;
+      if (FirstUserOfPhi1->getParent() != FirstUserOfPhi2->getParent())
+        return DT->dominates(FirstUserOfPhi1->getParent(),
+                             FirstUserOfPhi2->getParent());
+      auto *IE1 = dyn_cast<InsertElementInst>(FirstUserOfPhi1);
+      auto *IE2 = dyn_cast<InsertElementInst>(FirstUserOfPhi2);
+      auto *EE1 = dyn_cast<ExtractElementInst>(FirstUserOfPhi1);
+      auto *EE2 = dyn_cast<ExtractElementInst>(FirstUserOfPhi2);
+      if (IE1 && !IE2)
+        return true;
+      if (!IE1 && IE2)
+        return false;
+      if (IE1 && IE2) {
+        if (UserBVHead[I1] && !UserBVHead[I2])
+          return true;
+        if (!UserBVHead[I1])
+          return false;
+        if (UserBVHead[I1] == UserBVHead[I2])
           return getElementIndex(IE1) < getElementIndex(IE2);
-        }
-      if (auto *EE1 = dyn_cast<ExtractElementInst>(FirstUserOfPhi1))
-        if (auto *EE2 = dyn_cast<ExtractElementInst>(FirstUserOfPhi2)) {
-          if (EE1->getOperand(0) != EE2->getOperand(0))
-            return I1 < I2;
+        if (UserBVHead[I1]->getParent() != UserBVHead[I2]->getParent())
+          return DT->dominates(UserBVHead[I1]->getParent(),
+                               UserBVHead[I2]->getParent());
+        return UserBVHead[I1]->comesBefore(UserBVHead[I2]);
+      }
+      if (EE1 && !EE2)
+        return true;
+      if (!EE1 && EE2)
+        return false;
+      if (EE1 && EE2) {
+        if (EE1->getOperand(0) == EE2->getOperand(0))
           return getElementIndex(EE1) < getElementIndex(EE2);
+        auto *I1 = dyn_cast<Instruction>(EE1->getOperand(0));
+        if (I1 && !I2)
+          return true;
+        if (!I1 && I2)
+          return false;
+        auto *I2 = dyn_cast<Instruction>(EE2->getOperand(0));
+        if (I1 && I2) {
+          if (I1->getParent() != I2->getParent())
+            return DT->dominates(I1->getParent(), I2->getParent());
+          return I1->comesBefore(I2);
         }
-      return I1 < I2;
+        auto *P1 = dyn_cast<Argument>(EE1->getOperand(0));
+        auto *P2 = dyn_cast<Argument>(EE2->getOperand(0));
+        if (P1 && !P2)
+          return true;
+        if (!P1 && P2)
+          return false;
+        if (P1 && P2)
+          return P1->getArgNo() < P2->getArgNo();
+        // TODO: add analysis for other value kinds.
+        return EE1->getOperand(0)->getValueID() <
+               EE2->getOperand(0)->getValueID();
+      }
+      return false;
     };
     DenseMap<unsigned, unsigned> PhiToId;
     SmallVector<unsigned> Phis(TE.Scalars.size());
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
index dbc4f3d59d4f9b..d6073ea4bbbae6 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/reused-scalar-repeated-in-node.ll
@@ -33,40 +33,40 @@ define void @test() {
 ; CHECK-NEXT:    [[TMP7:%.*]] = insertelement <16 x float> [[TMP6]], float [[I68]], i32 6
 ; CHECK-NEXT:    [[TMP8:%.*]] = insertelement <16 x float> [[TMP7]], float [[I66]], i32 7
 ; CHECK-NEXT:    [[TMP9:%.*]] = insertelement <16 x float> [[TMP8]], float [[I72]], i32 13
-; CHECK-NEXT:    [[TMP10:%.*]] = insertelement <16 x float> [[TMP9]], float [[I69]], i32 14
-; CHECK-NEXT:    [[TMP11:%.*]] = insertelement <16 x float> [[TMP10]], float [[I67]], i32 15
+; CHECK-NEXT:    [[TMP10:%.*]] = insertelement <16 x float> [[TMP9]], float [[I67]], i32 14
+; CHECK-NEXT:    [[TMP11:%.*]] = insertelement <16 x float> [[TMP10]], float [[I69]], i32 15
 ; CHECK-NEXT:    br i1 poison, label %[[BB167:.*]], label %[[BB77:.*]]
 ; CHECK:       [[BB77]]:
-; CHECK-NEXT:    [[TMP12:%.*]] = shufflevector <16 x float> [[TMP11]], <16 x float> poison, <8 x i32> <i32 poison, i32 5, i32 6, i32 7, i32 15, i32 15, i32 14, i32 15>
+; CHECK-NEXT:    [[TMP12:%.*]] = shufflevector <16 x float> [[TMP11]], <16 x float> poison, <8 x i32> <i32 poison, i32 5, i32 6, i32 7, i32 14, i32 14, i32 14, i32 15>
 ; CHECK-NEXT:    [[TMP13:%.*]] = shufflevector <2 x float> [[TMP0]], <2 x float> poison, <16 x i32> <i32 poison, i32 poison, i32 1, i32 0, i32 0, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 1, i32 poison, i32 poison>
 ; CHECK-NEXT:    [[TMP14:%.*]] = shufflevector <2 x float> [[TMP0]], <2 x float> poison, <16 x i32> <i32 poison, i32 poison, i32 poison, i32 poison, i32 1, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 0, i32 poison, i32 poison>
 ; CHECK-NEXT:    br label %[[BB78:.*]]
 ; CHECK:       [[BB78]]:
 ; CHECK-NEXT:    [[TMP15:%.*]] = phi <8 x float> [ [[TMP12]], %[[BB77]] ], [ [[TMP30:%.*]], %[[BB78]] ]
 ; CHECK-NEXT:    [[TMP16:%.*]] = phi <2 x float> [ poison, %[[BB77]] ], [ [[TMP31:%.*]], %[[BB78]] ]
-; CHECK-NEXT:    [[TMP17:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 0, i32 3, i32 1, i32 2, i32 3, i32 0, i32 2, i32 3, i32 2, i32 7, i32 2, i32 3, i32 0, i32 6, i32 7, i32 7>
+; CHECK-NEXT:    [[TMP17:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 0, i32 3, i32 1, i32 2, i32 3, i32 0, i32 2, i32 3, i32 2, i32 6, i32 2, i32 3, i32 0, i32 7, i32 6, i32 6>
 ; CHECK-NEXT:    [[TMP18:%.*]] = fmul fast <16 x float> [[TMP17]], [[TMP13]]
-; CHECK-NEXT:    [[TMP19:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 1, i32 poison, i32 0, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 1, i32 7, i32 6, i32 6>
+; CHECK-NEXT:    [[TMP19:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 1, i32 poison, i32 0, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 1, i32 6, i32 7, i32 7>
 ; CHECK-NEXT:    [[TMP20:%.*]] = shufflevector <2 x float> [[TMP16]], <2 x float> poison, <16 x i32> <i32 0, i32 1, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
 ; CHECK-NEXT:    [[TMP21:%.*]] = shufflevector <16 x float> [[TMP19]], <16 x float> [[TMP20]], <16 x i32> <i32 0, i32 17, i32 2, i32 16, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 12, i32 13, i32 14, i32 15>
 ; CHECK-NEXT:    [[TMP22:%.*]] = shufflevector <8 x float> [[TMP15]], <8 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
-; CHECK-NEXT:    [[TMP23:%.*]] = shufflevector <16 x float> [[TMP21]], <16 x float> [[TMP22]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 17, i32 6, i32 7, i32 8, i32 22, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15>
+; CHECK-NEXT:    [[TMP23:%.*]] = shufflevector <16 x float> [[TMP21]], <16 x float> [[TMP22]], <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 17, i32 6, i32 7, i32 8, i32 23, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15>
 ; CHECK-NEXT:    [[TMP24:%.*]] = shufflevector <16 x float> [[TMP23]], <16 x float> poison, <16 x i32> <i32 0, i32 1, i32 2, i32 3, i32 1, i32 5, i32 3, i32 1, i32 3, i32 9, i32 3, i32 1, i32 12, i32 13, i32 14, i32 15>
 ; CHECK-NEXT:    [[TMP25:%.*]] = call <16 x float> @llvm.vector.insert.v16f32.v2f32(<16 x float> [[TMP14]], <2 x float> [[TMP0]], i64 2)
 ; CHECK-NEXT:    [[TMP26:%.*]] = fmul fast <16 x float> [[TMP24]], [[TMP25]]
 ; CHECK-NEXT:    [[TMP27:%.*]] = fadd fast <16 x float> [[TMP26]], [[TMP18]]
 ; CHECK-NEXT:    [[TMP28:%.*]] = fadd fast <16 x float> [[TMP27]], poison
 ; CHECK-NEXT:    [[TMP29:%.*]] = fadd fast <16 x float> [[TMP28]], poison
-; CHECK-NEXT:    [[TMP30]] = shufflevector <16 x float> [[TMP29]], <16 x float> poison, <8 x i32> <i32 12, i32 5, i32 6, i32 7, i32 15, i32 15, i32 14, i32 15>
+; CHECK-NEXT:    [[TMP30]] = shufflevector <16 x float> [[TMP29]], <16 x float> poison, <8 x i32> <i32 12, i32 5, i32 6, i32 7, i32 14, i32 14, i32 14, i32 15>
 ; CHECK-NEXT:    [[TMP31]] = shufflevector <16 x float> [[TMP29]], <16 x float> poison, <2 x i32> <i32 10, i32 11>
 ; CHECK-NEXT:    br i1 poison, label %[[BB78]], label %[[BB167]]
 ; CHECK:       [[BB167]]:
 ; CHECK-NEXT:    [[TMP32:%.*]] = phi <16 x float> [ [[TMP11]], %[[BB64]] ], [ [[TMP29]], %[[BB78]] ]
-; CHECK-NEXT:    [[TMP33:%.*]] = extractelement <16 x float> [[TMP32]], i32 15
+; CHECK-NEXT:    [[TMP33:%.*]] = extractelement <16 x float> [[TMP32]], i32 14
 ; CHECK-NEXT:    store float [[TMP33]], ptr poison, align 1
 ; CHECK-NEXT:    [[TMP34:%.*]] = extractelement <16 x float> [[TMP32]], i32 13
 ; CHECK-NEXT:    store float [[TMP34]], ptr poison, align 1
-; CHECK-NEXT:    [[TMP35:%.*]] = extractelement <16 x float> [[TMP32]], i32 14
+; CHECK-NEXT:    [[TMP35:%.*]] = extractelement <16 x float> [[TMP32]], i32 15
 ; CHECK-NEXT:    br i1 poison, label %[[BB186:.*]], label %[[BB184:.*]]
 ; CHECK:       [[BB184]]:
 ; CHECK-NEXT:    br label %[[BB185:.*]]
diff --git a/llvm/test/Transforms/SLPVectorizer/root-trunc-extract-reuse.ll b/llvm/test/Transforms/SLPVectorizer/root-trunc-extract-reuse.ll
index 34c068478c5f5e..d4b737a6bc4211 100644
--- a/llvm/test/Transforms/SLPVectorizer/root-trunc-extract-reuse.ll
+++ b/llvm/test/Transforms/SLPVectorizer/root-trunc-extract-reuse.ll
@@ -10,9 +10,9 @@ define i1 @test() {
 ; CHECK-NEXT:    br label [[ELSE]]
 ; CHECK:       else:
 ; CHECK-NEXT:    [[TMP0:%.*]] = phi <2 x i32> [ zeroinitializer, [[THEN]] ], [ zeroinitializer, [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[TMP1:%.*]] = extractelement <2 x i32> [[TMP0]], i32 0
+; CHECK-NEXT:    [[TMP1:%.*]] = extractelement <2 x i32> [[TMP0]], i32 1
 ; CHECK-NEXT:    [[BF_CAST162:%.*]] = and i32 [[TMP1]], 0
-; CHECK-NEXT:    [[TMP2:%.*]] = shufflevector <2 x i32> zeroinitializer, <2 x i32> [[TMP0]], <2 x i32> <i32 3, i32 1>
+; CHECK-NEXT:    [[TMP2:%.*]] = shufflevector <2 x i32> zeroinitializer, <2 x i32> [[TMP0]], <2 x i32> <i32 2, i32 1>
 ; CHECK-NEXT:    [[T13:%.*]] = and <2 x i32> [[TMP2]], zeroinitializer
 ; CHECK-NEXT:    br label [[ELSE1:%.*]]
 ; CHECK:       else1:

efriedma-quic
efriedma-quic reviewed Sep 30, 2024
View reviewed changes
@alexey-bataev
Rebase, address comments
09b3119 
Created using spr 1.3.5
efriedma-quic
efriedma-quic reviewed Oct 2, 2024
View reviewed changes
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
return P1->getArgNo() < P2->getArgNo();
return false;
}
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't quite work... if the first operand isn't an instruction or an argument, we have to always return false. So this needs to happen before the if (EE1->getOperand(0) == EE2->getOperand(0)).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it already does this. if (EE1->getOperand(0) == EE2->getOperand(0)) occurs only if both users are extractelements, argument comparison occurs only if both users are arguments. If the first is extract/argument but the second is not - return true. If the first is not extract/argument - return false. Am I missing something?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suppose you have three extractelements with a constant as the first operand. Two of them have A as the first operand, the other one has B as the first operand. They all need to compare equal, even if the extract indexes are different.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed it, fixed

@alexey-bataev
Rebase, rempve extra return
6c97b27 
Created using spr 1.3.5
@alexey-bataev
Rebase, address comments
4859bb7 
Created using spr 1.3.5
efriedma-quic
efriedma-quic reviewed Oct 2, 2024
View reviewed changes
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp Outdated
auto *Inst2 = dyn_cast<Instruction>(EE2->getOperand(0));
auto *P1 = dyn_cast<Argument>(EE1->getOperand(0));
auto *P2 = dyn_cast<Argument>(EE2->getOperand(0));
if ((!Inst1 && !Inst2) || (!P1 && !P2))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((!Inst1 && !Inst2) || (!P1 && !P2))
if (!Inst2 && !P2)
return Inst1 || P1;

@alexey-bataev
Address comments
cfaa773 
Created using spr 1.3.5
@alexey-bataev alexey-bataev mentioned this pull request Oct 4, 2024
Closed
efriedma-quic
efriedma-quic approved these changes Oct 4, 2024
View reviewed changes
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alexey-bataev alexey-bataev merged commit f74879c into main Oct 4, 2024
8 checks passed
@alexey-bataev alexey-bataev deleted the users/alexey-bataev/spr/slpmake-phicompare-comparator-follow-weak-strict-ordering-requirement branch October 4, 2024 18:23
@mikaelholmen
Copy link
Collaborator

Hi,

I bisected a crash back to this commit.
Reproduce with:
opt -passes="slp-vectorizer" -mtriple=aarch64 bbi-104215.ll -o /dev/null
Result:

opt: ../lib/Transforms/Vectorize/SLPVectorizer.cpp:5730: auto llvm::slpvectorizer::BoUpSLP::getReorderingData(const TreeEntry &, bool)::(anonymous class)::operator()(BasicBlock *, BasicBlock *) const: Assertion `NodeB && "Should only process reachable instructions"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all/bin/opt -passes=slp-vectorizer -mtriple=aarch64 bbi-104215.ll -o /dev/null
1.	Running pass "function(slp-vectorizer)" on module "bbi-104215.ll"
2.	Running pass "slp-vectorizer" on function "f_180_2259"
 #0 0x00005602853a5026 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all/bin/opt+0x4646026)
 #1 0x00005602853a2a6e llvm::sys::RunSignalHandlers() (build-all/bin/opt+0x4643a6e)
 #2 0x00005602853a58a9 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007f5db37acd10 __restore_rt (/lib64/libpthread.so.0+0x12d10)
 #4 0x00007f5db114c52f raise (/lib64/libc.so.6+0x4e52f)
 #5 0x00007f5db111fe65 abort (/lib64/libc.so.6+0x21e65)
 #6 0x00007f5db111fd39 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d39)
 #7 0x00007f5db1144e86 (/lib64/libc.so.6+0x46e86)
 #8 0x0000560286e891cd llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_6::operator()(llvm::BasicBlock*, llvm::BasicBlock*) const SLPVectorizer.cpp:0:0
 #9 0x0000560286e8887d void std::__merge_adaptive<unsigned int*, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>>(unsigned int*, unsigned int*, unsigned int*, long, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>) SLPVectorizer.cpp:0:0
#10 0x0000560286dd42b3 llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool) (build-all/bin/opt+0x60752b3)
#11 0x0000560286dd8354 llvm::slpvectorizer::BoUpSLP::reorderTopToBottom() (build-all/bin/opt+0x6079354)
#12 0x0000560286e3ff0c llvm::SLPVectorizerPass::tryToVectorizeList(llvm::ArrayRef<llvm::Value*>, llvm::slpvectorizer::BoUpSLP&, bool) (build-all/bin/opt+0x60e0f0c)
#13 0x0000560286e45a04 bool tryToVectorizeSequence<llvm::Value>(llvm::SmallVectorImpl<llvm::Value*>&, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::ArrayRef<llvm::Value*>, bool)>, bool, llvm::slpvectorizer::BoUpSLP&) SLPVectorizer.cpp:0:0
#14 0x0000560286e3819e llvm::SLPVectorizerPass::vectorizeChainsInBlock(llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&) (build-all/bin/opt+0x60d919e)
#15 0x0000560286e35c64 llvm::SLPVectorizerPass::runImpl(llvm::Function&, llvm::ScalarEvolution*, llvm::TargetTransformInfo*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::DemandedBits*, llvm::OptimizationRemarkEmitter*) (build-all/bin/opt+0x60d6c64)
#16 0x0000560286e351e7 llvm::SLPVectorizerPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x60d61e7)
#17 0x000056028682b6cd llvm::detail::PassModel<llvm::Function, llvm::SLPVectorizerPass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#18 0x00005602855c75c7 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x48685c7)
#19 0x00005602868220fd llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#20 0x00005602855cc19e llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x486d19e)
#21 0x00005602868187ed llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) PassBuilderPipelines.cpp:0:0
#22 0x00005602855c62b7 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x48672b7)
#23 0x00005602867a3b5c llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (build-all/bin/opt+0x5a44b5c)
#24 0x0000560285367c12 optMain (build-all/bin/opt+0x4608c12)
#25 0x00007f5db11387e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
#26 0x000056028536582e _start (build-all/bin/opt+0x460682e)
Abort (core dumped)

bbi-104215.ll.gz

alexey-bataev added a commit that referenced this pull request Feb 18, 2025
@alexey-bataev
[SLP]Fix a crash when comparing phis from unreachable blocks
0e1ffa3 
Need to check if the block is reachable before comparing phis from it to
avoid compiler crash when requesting node.

Fixes report in #110529 (comment)
@alexey-bataev
Copy link
Member Author

Hi,

I bisected a crash back to this commit. Reproduce with: opt -passes="slp-vectorizer" -mtriple=aarch64 bbi-104215.ll -o /dev/null Result:

opt: ../lib/Transforms/Vectorize/SLPVectorizer.cpp:5730: auto llvm::slpvectorizer::BoUpSLP::getReorderingData(const TreeEntry &, bool)::(anonymous class)::operator()(BasicBlock *, BasicBlock *) const: Assertion `NodeB && "Should only process reachable instructions"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all/bin/opt -passes=slp-vectorizer -mtriple=aarch64 bbi-104215.ll -o /dev/null
1.	Running pass "function(slp-vectorizer)" on module "bbi-104215.ll"
2.	Running pass "slp-vectorizer" on function "f_180_2259"
 #0 0x00005602853a5026 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all/bin/opt+0x4646026)
 #1 0x00005602853a2a6e llvm::sys::RunSignalHandlers() (build-all/bin/opt+0x4643a6e)
 #2 0x00005602853a58a9 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007f5db37acd10 __restore_rt (/lib64/libpthread.so.0+0x12d10)
 #4 0x00007f5db114c52f raise (/lib64/libc.so.6+0x4e52f)
 #5 0x00007f5db111fe65 abort (/lib64/libc.so.6+0x21e65)
 #6 0x00007f5db111fd39 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d39)
 #7 0x00007f5db1144e86 (/lib64/libc.so.6+0x46e86)
 #8 0x0000560286e891cd llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_6::operator()(llvm::BasicBlock*, llvm::BasicBlock*) const SLPVectorizer.cpp:0:0
 #9 0x0000560286e8887d void std::__merge_adaptive<unsigned int*, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>>(unsigned int*, unsigned int*, unsigned int*, long, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>) SLPVectorizer.cpp:0:0
#10 0x0000560286dd42b3 llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool) (build-all/bin/opt+0x60752b3)
#11 0x0000560286dd8354 llvm::slpvectorizer::BoUpSLP::reorderTopToBottom() (build-all/bin/opt+0x6079354)
#12 0x0000560286e3ff0c llvm::SLPVectorizerPass::tryToVectorizeList(llvm::ArrayRef<llvm::Value*>, llvm::slpvectorizer::BoUpSLP&, bool) (build-all/bin/opt+0x60e0f0c)
#13 0x0000560286e45a04 bool tryToVectorizeSequence<llvm::Value>(llvm::SmallVectorImpl<llvm::Value*>&, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::ArrayRef<llvm::Value*>, bool)>, bool, llvm::slpvectorizer::BoUpSLP&) SLPVectorizer.cpp:0:0
#14 0x0000560286e3819e llvm::SLPVectorizerPass::vectorizeChainsInBlock(llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&) (build-all/bin/opt+0x60d919e)
#15 0x0000560286e35c64 llvm::SLPVectorizerPass::runImpl(llvm::Function&, llvm::ScalarEvolution*, llvm::TargetTransformInfo*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::DemandedBits*, llvm::OptimizationRemarkEmitter*) (build-all/bin/opt+0x60d6c64)
#16 0x0000560286e351e7 llvm::SLPVectorizerPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x60d61e7)
#17 0x000056028682b6cd llvm::detail::PassModel<llvm::Function, llvm::SLPVectorizerPass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#18 0x00005602855c75c7 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x48685c7)
#19 0x00005602868220fd llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#20 0x00005602855cc19e llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x486d19e)
#21 0x00005602868187ed llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) PassBuilderPipelines.cpp:0:0
#22 0x00005602855c62b7 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x48672b7)
#23 0x00005602867a3b5c llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (build-all/bin/opt+0x5a44b5c)
#24 0x0000560285367c12 optMain (build-all/bin/opt+0x4608c12)
#25 0x00007f5db11387e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
#26 0x000056028536582e _start (build-all/bin/opt+0x460682e)
Abort (core dumped)

bbi-104215.ll.gz

Must be fixed in 0e1ffa3

github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 18, 2025
@alexey-bataev @github-actions
Automerge: [SLP]Fix a crash when comparing phis from unreachable blocks
7b899f1 
Need to check if the block is reachable before comparing phis from it to
avoid compiler crash when requesting node.

Fixes report in llvm/llvm-project#110529 (comment)
@mikaelholmen
Copy link
Collaborator

Hi,
I bisected a crash back to this commit. Reproduce with: opt -passes="slp-vectorizer" -mtriple=aarch64 bbi-104215.ll -o /dev/null Result:

opt: ../lib/Transforms/Vectorize/SLPVectorizer.cpp:5730: auto llvm::slpvectorizer::BoUpSLP::getReorderingData(const TreeEntry &, bool)::(anonymous class)::operator()(BasicBlock *, BasicBlock *) const: Assertion `NodeB && "Should only process reachable instructions"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all/bin/opt -passes=slp-vectorizer -mtriple=aarch64 bbi-104215.ll -o /dev/null
1.	Running pass "function(slp-vectorizer)" on module "bbi-104215.ll"
2.	Running pass "slp-vectorizer" on function "f_180_2259"
 #0 0x00005602853a5026 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all/bin/opt+0x4646026)
 #1 0x00005602853a2a6e llvm::sys::RunSignalHandlers() (build-all/bin/opt+0x4643a6e)
 #2 0x00005602853a58a9 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007f5db37acd10 __restore_rt (/lib64/libpthread.so.0+0x12d10)
 #4 0x00007f5db114c52f raise (/lib64/libc.so.6+0x4e52f)
 #5 0x00007f5db111fe65 abort (/lib64/libc.so.6+0x21e65)
 #6 0x00007f5db111fd39 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d39)
 #7 0x00007f5db1144e86 (/lib64/libc.so.6+0x46e86)
 #8 0x0000560286e891cd llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_6::operator()(llvm::BasicBlock*, llvm::BasicBlock*) const SLPVectorizer.cpp:0:0
 #9 0x0000560286e8887d void std::__merge_adaptive<unsigned int*, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>>(unsigned int*, unsigned int*, unsigned int*, long, long, unsigned int*, __gnu_cxx::__ops::_Iter_comp_iter<llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool)::$_7>) SLPVectorizer.cpp:0:0
#10 0x0000560286dd42b3 llvm::slpvectorizer::BoUpSLP::getReorderingData(llvm::slpvectorizer::BoUpSLP::TreeEntry const&, bool) (build-all/bin/opt+0x60752b3)
#11 0x0000560286dd8354 llvm::slpvectorizer::BoUpSLP::reorderTopToBottom() (build-all/bin/opt+0x6079354)
#12 0x0000560286e3ff0c llvm::SLPVectorizerPass::tryToVectorizeList(llvm::ArrayRef<llvm::Value*>, llvm::slpvectorizer::BoUpSLP&, bool) (build-all/bin/opt+0x60e0f0c)
#13 0x0000560286e45a04 bool tryToVectorizeSequence<llvm::Value>(llvm::SmallVectorImpl<llvm::Value*>&, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::Value*, llvm::Value*)>, llvm::function_ref<bool (llvm::ArrayRef<llvm::Value*>, bool)>, bool, llvm::slpvectorizer::BoUpSLP&) SLPVectorizer.cpp:0:0
#14 0x0000560286e3819e llvm::SLPVectorizerPass::vectorizeChainsInBlock(llvm::BasicBlock*, llvm::slpvectorizer::BoUpSLP&) (build-all/bin/opt+0x60d919e)
#15 0x0000560286e35c64 llvm::SLPVectorizerPass::runImpl(llvm::Function&, llvm::ScalarEvolution*, llvm::TargetTransformInfo*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::DemandedBits*, llvm::OptimizationRemarkEmitter*) (build-all/bin/opt+0x60d6c64)
#16 0x0000560286e351e7 llvm::SLPVectorizerPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x60d61e7)
#17 0x000056028682b6cd llvm::detail::PassModel<llvm::Function, llvm::SLPVectorizerPass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#18 0x00005602855c75c7 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x48685c7)
#19 0x00005602868220fd llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#20 0x00005602855cc19e llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x486d19e)
#21 0x00005602868187ed llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) PassBuilderPipelines.cpp:0:0
#22 0x00005602855c62b7 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x48672b7)
#23 0x00005602867a3b5c llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (build-all/bin/opt+0x5a44b5c)
#24 0x0000560285367c12 optMain (build-all/bin/opt+0x4608c12)
#25 0x00007f5db11387e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
#26 0x000056028536582e _start (build-all/bin/opt+0x460682e)
Abort (core dumped)

bbi-104215.ll.gz

Must be fixed in 0e1ffa3

Yep, thanks.

wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this pull request Feb 19, 2025
@alexey-bataev @wldfngrs
[SLP]Fix a crash when comparing phis from unreachable blocks
5ab333a 
Need to check if the block is reachable before comparing phis from it to
avoid compiler crash when requesting node.

Fixes report in llvm#110529 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efriedma-quic efriedma-quic efriedma-quic approved these changes

Assignees
No one assigned
Labels
llvm:transforms vectorizers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexey-bataev @llvmbot @mikaelholmen @efriedma-quic