[llvm] a1a9c53 - [GlobalISel] Fix infinite loop in reassociation combine

[Jay Foad via llvm-commits]

Author: Jay Foad
Date: 2023-07-16T14:15:24+01:00
New Revision: a1a9c53ae7f13ecfec588bc17bcbb75a7c9c3c16

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

LOG: [GlobalISel] Fix infinite loop in reassociation combine

Don't reassociate (C1+C2)+Y -> C1+(C2+Y).

Fixes https://github.com/llvm/llvm-project/issues/63849

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

Added: 
    llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-reassoc.mir

Modified: 
    llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index e094d644fd3a1c..cc7fb3ee110913 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -4475,7 +4475,12 @@ bool CombinerHelper::tryReassocBinOp(unsigned Opc, Register DstReg,
   Register OpLHSLHS = OpLHSDef->getOperand(1).getReg();
   Register OpLHSRHS = OpLHSDef->getOperand(2).getReg();
 
-  if (isConstantOrConstantSplatVector(*MRI.getVRegDef(OpLHSRHS), MRI)) {
+  // If the inner op is (X op C), pull the constant out so it can be folded with
+  // other constants in the expression tree. Folding is not guaranteed so we
+  // might have (C1 op C2). In that case do not pull a constant out because it
+  // won't help and can lead to infinite loops.
+  if (isConstantOrConstantSplatVector(*MRI.getVRegDef(OpLHSRHS), MRI) &&
+      !isConstantOrConstantSplatVector(*MRI.getVRegDef(OpLHSLHS), MRI)) {
     if (isConstantOrConstantSplatVector(*OpRHSDef, MRI)) {
       // (Opc (Opc X, C1), C2) -> (Opc X, (Opc C1, C2))
       MatchInfo = [=](MachineIRBuilder &B) {

diff  --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-reassoc.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-reassoc.mir
new file mode 100644
index 00000000000000..51f337dde407f4
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-reassoc.mir
@@ -0,0 +1,30 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+# RUN: llc -march=amdgcn -run-pass=amdgpu-postlegalizer-combiner -verify-machineinstrs -o - %s | FileCheck %s
+
+---
+name: test_reassoc_infinite_loop
+legalized: true
+tracksRegLiveness: true
+body:             |
+  bb.1:
+    liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4
+
+    ; CHECK-LABEL: name: test_reassoc_infinite_loop
+    ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3, $vgpr4
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 2
+    ; CHECK-NEXT: [[ADD:%[0-9]+]]:_(s32) = G_ADD [[COPY]], [[C]]
+    ; CHECK-NEXT: $vgpr0 = COPY [[ADD]](s32)
+    ; CHECK-NEXT: SI_RETURN implicit $vgpr0
+    %0:_(s32) = COPY $vgpr0
+    %1:_(s32) = G_CONSTANT i32 0
+    %2:_(s32) = G_CONSTANT i32 1
+    %3:_(s1) = G_ICMP intpred(eq), %1(s32), %1
+    %4:_(s32) = G_SELECT %3(s1), %2, %1
+    %5:_(s32) = COPY %4(s32)
+    %6:_(s32) = G_ADD %0, %5
+    %7:_(s32) = G_ADD %6, %2
+    $vgpr0 = COPY %7(s32)
+    SI_RETURN implicit $vgpr0
+...