[llvm] Fix crash caused by reprocessing complex reductions

[NickGuy-Arm]

If a complex pattern had the shape of both a complex->complex reduction and a complex->single reduction, the matching would recognise both and deem the graph a valid transformation. Preventing this reprocessing results in only one of these matching, meaning that in the case of an invalid graph, we don't try to transform it anyway.

[llvmbot]

@llvm/pr-subscribers-backend-aarch64

Author: Nicholas Guy (NickGuy-Arm)

Changes

If a complex pattern had the shape of both a complex->complex reduction and a complex->single reduction, the matching would recognise both and deem the graph a valid transformation. Preventing this reprocessing results in only one of these matching, meaning that in the case of an invalid graph, we don't try to transform it anyway.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp (+1-1)
• (added) llvm/test/CodeGen/AArch64/complex-deinterleaving-opt-crash.ll (+35)

diff --git a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
index aec8df962ffb7c..92053ed5619010 100644
--- a/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
+++ b/llvm/lib/CodeGen/ComplexDeinterleavingPass.cpp
@@ -1730,7 +1730,7 @@ void ComplexDeinterleavingGraph::identifyReductionNodes() {
     auto *Real = OperationInstruction[i];
     // We want to check that we have 2 operands, but the function attributes
     // being counted as operands bloats this value.
-    if (Real->getNumOperands() < 2)
+    if (Processed[i] || Real->getNumOperands() < 2)
       continue;
 
     RealPHI = ReductionInfo[Real].first;
diff --git a/llvm/test/CodeGen/AArch64/complex-deinterleaving-opt-crash.ll b/llvm/test/CodeGen/AArch64/complex-deinterleaving-opt-crash.ll
new file mode 100644
index 00000000000000..2fc6f6c8860543
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/complex-deinterleaving-opt-crash.ll
@@ -0,0 +1,35 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S --passes=complex-deinterleaving %s --mattr=+sve2 | FileCheck %s
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-ni:1-p2:32:8:8:32-ni:2"
+target triple = "aarch64-arm-none-linux"
+
+; Ensure that a second reduction-like pattern doesn't override the first
+; We don't care what this IR produces, just that it produces something and doesn't cause a crash
+define void @reprocessing_crash() #0 {
+; CHECK-LABEL: define void @reprocessing_crash
+;
+entry:
+  br label %vector.body
+
+vector.body:                                      ; preds = %vector.body, %entry
+  %vec.phi18 = phi <vscale x 2 x double> [ zeroinitializer, %entry ], [ %2, %vector.body ]
+  %vec.phi20 = phi <vscale x 2 x double> [ zeroinitializer, %entry ], [ %3, %vector.body ]
+  %strided.vec22 = tail call { <vscale x 2 x double>, <vscale x 2 x double> } @llvm.vector.deinterleave2.nxv4f64(<vscale x 4 x double> zeroinitializer)
+  %0 = extractvalue { <vscale x 2 x double>, <vscale x 2 x double> } %strided.vec22, 0
+  %1 = extractvalue { <vscale x 2 x double>, <vscale x 2 x double> } %strided.vec22, 1
+  %2 = fsub <vscale x 2 x double> %vec.phi18, %0
+  %3 = fsub <vscale x 2 x double> %vec.phi20, %1
+  br i1 false, label %middle.block, label %vector.body
+
+middle.block:                                     ; preds = %vector.body
+  %bin.rdx = fadd <vscale x 2 x double> %2, zeroinitializer
+  %bin.rdx23 = fadd <vscale x 2 x double> %3, zeroinitializer
+  ret void
+}
+
+; Function Attrs: nocallback nofree nosync nounwind willreturn memory(none)
+declare { <vscale x 2 x double>, <vscale x 2 x double> } @llvm.vector.deinterleave2.nxv4f64(<vscale x 4 x double>) #1
+
+attributes #0 = { "target-cpu"="neoverse-v1" }
+attributes #1 = { nocallback nofree nosync nounwind willreturn memory(none) }

[DavidSpickett]

I understand that the motivation for this fix is a Fujitsu test suite regression caught by Linaro (https://linaro.atlassian.net/browse/LLVM-1501), I have pinged the assignee there to check this.

If in the next few days there is another strong reason to go ahead and land this, please do so. Linaro will follow up if there's anything still failing afterwards.

[DavidSpickett]

You can go ahead and land this once your reviewers are happy. Linaro will monitor the tests.

(and the obviously unrelated CI failure, I just saw a fix come by for that anyway)