[llvm] d71ad41 - [SLP]Fix insertpoint of the extractellements instructions to avoid reshuffle crash.

[Alexey Bataev via llvm-commits]

Author: Alexey Bataev
Date: 2022-10-12T08:18:30-07:00
New Revision: d71ad4108056d685f48407447095d8d92fd7685d

URL: https://github.com/llvm/llvm-project/commit/d71ad4108056d685f48407447095d8d92fd7685d
DIFF: https://github.com/llvm/llvm-project/commit/d71ad4108056d685f48407447095d8d92fd7685d.diff

LOG: [SLP]Fix insertpoint of the extractellements instructions to avoid reshuffle crash.

Need to set the insertpoint for extractelement to point to the first
instruction in the node to avoid possible crash during external uses
combine  process. Without it we may endup with the incorrect
transformation.

Differential Revision: https://reviews.llvm.org/D135591

Added: 
    llvm/test/Transforms/SLPVectorizer/X86/extractelement-insertpoint.ll

Modified: 
    llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 10d9293d928e..865578f7c4f0 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -7693,13 +7693,34 @@ Instruction &BoUpSLP::getLastInstructionInBundle(const TreeEntry *E) {
     return LastInst;
   };
 
-  auto &&FindFirstInst = [E, Front]() {
+  auto &&FindFirstInst = [E, Front, this]() {
     Instruction *FirstInst = Front;
     for (Value *V : E->Scalars) {
       auto *I = dyn_cast<Instruction>(V);
       if (!I)
         continue;
-      if (I->comesBefore(FirstInst))
+      if (FirstInst->getParent() == I->getParent()) {
+        if (I->comesBefore(FirstInst))
+          FirstInst = I;
+        continue;
+      }
+      assert(isVectorLikeInstWithConstOps(FirstInst) &&
+             isVectorLikeInstWithConstOps(I) &&
+             "Expected vector-like insts only.");
+      if (!DT->isReachableFromEntry(FirstInst->getParent())) {
+        FirstInst = I;
+        continue;
+      }
+      if (!DT->isReachableFromEntry(I->getParent()))
+        continue;
+      auto *NodeA = DT->getNode(FirstInst->getParent());
+      auto *NodeB = DT->getNode(I->getParent());
+      assert(NodeA && "Should only process reachable instructions");
+      assert(NodeB && "Should only process reachable instructions");
+      assert((NodeA == NodeB) ==
+                 (NodeA->getDFSNumIn() == NodeB->getDFSNumIn()) &&
+             "Different nodes should have 
diff erent DFS numbers");
+      if (NodeA->getDFSNumIn() > NodeB->getDFSNumIn())
         FirstInst = I;
     }
     return FirstInst;
@@ -7708,9 +7729,12 @@ Instruction &BoUpSLP::getLastInstructionInBundle(const TreeEntry *E) {
   // Set the insert point to the beginning of the basic block if the entry
   // should not be scheduled.
   if (E->State != TreeEntry::NeedToGather &&
-      doesNotNeedToSchedule(E->Scalars)) {
+      (doesNotNeedToSchedule(E->Scalars) ||
+       all_of(E->Scalars, isVectorLikeInstWithConstOps))) {
     Instruction *InsertInst;
-    if (all_of(E->Scalars, isUsedOutsideBlock))
+    if (all_of(E->Scalars, [](Value *V) {
+          return !isVectorLikeInstWithConstOps(V) && isUsedOutsideBlock(V);
+        }))
       InsertInst = FindLastInst();
     else
       InsertInst = FindFirstInst();
@@ -8135,7 +8159,7 @@ Value *BoUpSLP::vectorizeTree(TreeEntry *E) {
 
     case Instruction::ExtractElement: {
       Value *V = E->getSingleOperand(0);
-      Builder.SetInsertPoint(VL0);
+      setInsertPointAfterBundle(E);
       ShuffleBuilder.addInversedMask(E->ReorderIndices);
       ShuffleBuilder.addMask(E->ReuseShuffleIndices);
       V = ShuffleBuilder.finalize(V);

diff  --git a/llvm/test/Transforms/SLPVectorizer/X86/extractelement-insertpoint.ll b/llvm/test/Transforms/SLPVectorizer/X86/extractelement-insertpoint.ll
new file mode 100644
index 000000000000..0d53121c9dee
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/X86/extractelement-insertpoint.ll
@@ -0,0 +1,31 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -slp-vectorizer -mtriple=x86_64-grtev4-linux-gnu -o - < %s | FileCheck %s
+
+define i32 @crash() {
+; CHECK-LABEL: @crash(
+; CHECK-NEXT:  label:
+; CHECK-NEXT:    [[ADD:%.*]] = fadd <2 x double> zeroinitializer, zeroinitializer
+; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <2 x double> [[ADD]], <2 x double> poison, <2 x i32> <i32 1, i32 0>
+; CHECK-NEXT:    [[TMP0:%.*]] = shufflevector <2 x double> zeroinitializer, <2 x double> [[SHUFFLE]], <2 x i32> <i32 2, i32 1>
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul <2 x double> [[SHUFFLE]], zeroinitializer
+; CHECK-NEXT:    [[TMP2:%.*]] = extractelement <2 x double> [[TMP1]], i32 0
+; CHECK-NEXT:    [[TMP3:%.*]] = extractelement <2 x double> [[TMP1]], i32 1
+; CHECK-NEXT:    [[ADD1:%.*]] = fadd double [[TMP2]], [[TMP3]]
+; CHECK-NEXT:    [[MUL1:%.*]] = fmul double [[ADD1]], 0.000000e+00
+; CHECK-NEXT:    store double [[MUL1]], double* null, align 16
+; CHECK-NEXT:    ret i32 0
+;
+label:
+  %0 = extractelement <2 x double> zeroinitializer, i64 1
+  %1 = extractelement <2 x double> zeroinitializer, i64 0
+  %add = fadd <2 x double> zeroinitializer, zeroinitializer
+  %extract1 = extractelement <2 x double> %add, i64 1
+  %2 = fmul double %extract1, %1
+  %insert = insertelement <2 x double> zeroinitializer, double %extract1, i64 0
+  %extract0 = extractelement <2 x double> %add, i64 0
+  %mul = fmul double %extract0, %0
+  %add1 = fadd double %2, %mul
+  %mul1 = fmul double %add1, 0.000000e+00
+  store double %mul1, double* null, align 16
+  ret i32 0
+}