[GlobalISel] Fix infinite loop in reassociation combine

[foad]

Summary

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

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

[arsenm]

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

3

don't need -global-isel

22

should run-pass=none to compact the register numbers, it's sometimes annoying when the register numbers actually tested don't match the ones in the file

[foad]

Update test.

[aemerson]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4478–4484

Ca you add a comment here to explain why we do the second constant check.

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

22

TIL

[foad]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4478–4484

I tried, but I'm not super confident about the explanation for why we are doing these reassociations in the first place.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

bool CombinerHelper::tryReassocBinOp(unsigned Opc, Register DstReg,
 
   if (OpLHSDef->getOpcode() != Opc)
     return false;
 
   MachineInstr *OpRHSDef = MRI.getVRegDef(OpRHS);
   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) {
         auto NewCst = B.buildInstr(Opc, {OpRHSTy}, {OpLHSRHS, OpRHS});
         B.buildInstr(Opc, {DstReg}, {OpLHSLHS, NewCst});
       };
       return true;
     }
     if (getTargetLowering().isReassocProfitable(MRI, OpLHS, OpRHS)) {

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

+# 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
+...