[llvm] [SLP]Fix dominance crash for scheduled copyable PHI-operand bundles (PR #205372)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-vectorizers

Author: Alexey Bataev (alexey-bataev)

<details>
<summary>Changes</summary>

Extend the copyable/non-copyable PHI conflict bail-out in
tryScheduleBundle to the scheduled path, not just the non-schedulable
one.

Fixes #<!-- -->205327


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


2 Files Affected:

- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+21-12) 
- (added) llvm/test/Transforms/SLPVectorizer/X86/copyable-phi-scheduled-non-copyable.ll (+47) 


``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 52566c094f6a6..1d7e8e5ddf50f 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -25549,6 +25549,27 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
         return std::nullopt;
     }
   }
+  // A scalar that is a non-copyable (i.e. really vectorized) element in this
+  // PHI-operand node, but a copyable element in another PHI-operand node, is
+  // vectorized here and only gathered/reused there. Both nodes feed different
+  // incoming values of the same vectorized PHI and may be emitted in different
+  // predecessor blocks, so the gathered reuse can end up not dominated by the
+  // vectorized value, producing broken IR. Bail out of scheduling to avoid it.
+  if (EI && EI.UserTE->State == TreeEntry::Vectorize &&
+      EI.UserTE->getOpcode() == Instruction::PHI && any_of(VL, [&](Value *V) {
+        auto *I = dyn_cast<Instruction>(V);
+        if (!I || (HasCopyables && S.isCopyableElement(V)))
+          return false;
+        return any_of(
+            SLP->VectorizableTree, [&](const std::unique_ptr<TreeEntry> &TE) {
+              return TE->UserTreeIndex &&
+                     TE->UserTreeIndex.UserTE->State == TreeEntry::Vectorize &&
+                     TE->UserTreeIndex.UserTE->getOpcode() ==
+                         Instruction::PHI &&
+                     TE->hasCopyableElements() && TE->isCopyableElement(V);
+            });
+      }))
+    return std::nullopt;
   if (DoesNotRequireScheduling) {
     // If all operands were replaced by copyables, the operands of this node
     // might be not, so need to recalculate dependencies for schedule data,
@@ -25579,18 +25600,6 @@ BoUpSLP::BlockScheduling::tryScheduleBundle(ArrayRef<Value *> VL, BoUpSLP *SLP,
             return SD && SD->hasValidDependencies();
           }))
         return std::nullopt;
-      if (EI && EI.UserTE->State == TreeEntry::Vectorize &&
-          EI.UserTE->getOpcode() == Instruction::PHI &&
-          any_of(SLP->VectorizableTree,
-                 [&](const std::unique_ptr<TreeEntry> &TE) {
-                   return TE->UserTreeIndex &&
-                          TE->UserTreeIndex.UserTE->State ==
-                              TreeEntry::Vectorize &&
-                          TE->UserTreeIndex.UserTE->getOpcode() ==
-                              Instruction::PHI &&
-                          TE->hasCopyableElements() && TE->isCopyableElement(V);
-                 }))
-        return std::nullopt;
       SmallDenseMap<std::pair<Instruction *, Value *>, unsigned> UserOpToNumOps;
       for (const Use &U : I->operands()) {
         unsigned &NumOps =
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/copyable-phi-scheduled-non-copyable.ll b/llvm/test/Transforms/SLPVectorizer/X86/copyable-phi-scheduled-non-copyable.ll
new file mode 100644
index 0000000000000..762366cb3b623
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/X86/copyable-phi-scheduled-non-copyable.ll
@@ -0,0 +1,47 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -S --passes=slp-vectorizer -mtriple=x86_64-unknown-linux-gnu -slp-threshold=-99999 < %s | FileCheck %s
+
+define void @test(i32 %arg) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: i32 [[ARG:%.*]]) {
+; CHECK-NEXT:  [[BB:.*:]]
+; CHECK-NEXT:    br label %[[BB1:.*]]
+; CHECK:       [[BB1]]:
+; CHECK-NEXT:    [[TMP0:%.*]] = insertelement <2 x i32> <i32 poison, i32 0>, i32 [[ARG]], i32 0
+; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <2 x i32> [[TMP0]], <2 x i32> <i32 0, i32 poison>, <2 x i32> <i32 2, i32 0>
+; CHECK-NEXT:    [[TMP2:%.*]] = sub <2 x i32> [[TMP0]], [[TMP1]]
+; CHECK-NEXT:    [[TMP3:%.*]] = shufflevector <2 x i32> [[TMP2]], <2 x i32> poison, <4 x i32> <i32 0, i32 poison, i32 1, i32 poison>
+; CHECK-NEXT:    [[TMP4:%.*]] = shufflevector <4 x i32> <i32 poison, i32 1, i32 poison, i32 0>, <4 x i32> [[TMP3]], <4 x i32> <i32 4, i32 1, i32 6, i32 3>
+; CHECK-NEXT:    [[TMP5:%.*]] = add <4 x i32> zeroinitializer, [[TMP4]]
+; CHECK-NEXT:    [[ICMP:%.*]] = icmp sgt i32 0, 0
+; CHECK-NEXT:    [[TMP6:%.*]] = shufflevector <4 x i32> [[TMP5]], <4 x i32> <i32 poison, i32 poison, i32 poison, i32 1>, <4 x i32> <i32 0, i32 1, i32 2, i32 7>
+; CHECK-NEXT:    [[TMP7:%.*]] = insertelement <4 x i32> [[TMP6]], i32 0, i32 1
+; CHECK-NEXT:    br i1 false, label %[[BB5:.*]], label %[[BB4:.*]]
+; CHECK:       [[BB4]]:
+; CHECK-NEXT:    br label %[[BB5]]
+; CHECK:       [[BB5]]:
+; CHECK-NEXT:    [[TMP8:%.*]] = phi <4 x i32> [ [[TMP5]], %[[BB4]] ], [ [[TMP7]], %[[BB1]] ]
+; CHECK-NEXT:    ret void
+;
+bb:
+  br label %bb1
+
+bb1:
+  %sub = sub i32 0, %arg
+  %or = or i32 %arg, 0
+  %add = add i32 %or, 0
+  %add2 = add i32 0, 0
+  %icmp = icmp sgt i32 %add2, 0
+  %add3 = add i32 0, 1
+  br i1 false, label %bb5, label %bb4
+
+bb4:
+  br label %bb5
+
+bb5:
+  %phi = phi i32 [ 0, %bb4 ], [ 1, %bb1 ]
+  %phi6 = phi i32 [ %sub, %bb4 ], [ %sub, %bb1 ]
+  %phi7 = phi i32 [ %add, %bb4 ], [ %add, %bb1 ]
+  %phi8 = phi i32 [ %add3, %bb4 ], [ %add2, %bb1 ]
+  ret void
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/205372