From 1f01c580444ea2daef67f95ffc5fde2de5a37cec Mon Sep 17 00:00:00 2001 From: darkbuck Date: Wed, 3 Apr 2024 20:52:21 -0400 Subject: [GlobalISel] Fix the infinite loop issue in `commute_int_constant_to_rhs` - When both operands are constant, the matcher runs into an infinite loop as the commutation should be applied only when LHS is a constant and RHS is not. Reviewers: arsenm Reviewed By: arsenm Pull Request: https://github.com/llvm/llvm-project/pull/87426 --- llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 17 ++++++------- .../GlobalISel/combine-commute-int-const-lhs.mir | 28 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/GlobalISel/combine-commute-int-const-lhs.mir diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp index 5cf7a33..062132c 100644 --- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp @@ -6276,14 +6276,15 @@ bool CombinerHelper::matchShiftsTooBig(MachineInstr &MI) { bool CombinerHelper::matchCommuteConstantToRHS(MachineInstr &MI) { Register LHS = MI.getOperand(1).getReg(); Register RHS = MI.getOperand(2).getReg(); - auto *LHSDef = MRI.getVRegDef(LHS); - if (getIConstantVRegVal(LHS, MRI).has_value()) - return true; - - // LHS may be a G_CONSTANT_FOLD_BARRIER. If so we commute - // as long as we don't already have a constant on the RHS. - if (LHSDef->getOpcode() != TargetOpcode::G_CONSTANT_FOLD_BARRIER) - return false; + if (!getIConstantVRegVal(LHS, MRI)) { + // Skip commuting if LHS is not a constant. But, LHS may be a + // G_CONSTANT_FOLD_BARRIER. If so we commute as long as we don't already + // have a constant on the RHS. + if (MRI.getVRegDef(LHS)->getOpcode() != + TargetOpcode::G_CONSTANT_FOLD_BARRIER) + return false; + } + // Commute as long as RHS is not a constant or G_CONSTANT_FOLD_BARRIER. return MRI.getVRegDef(RHS)->getOpcode() != TargetOpcode::G_CONSTANT_FOLD_BARRIER && !getIConstantVRegVal(RHS, MRI); diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-commute-int-const-lhs.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-commute-int-const-lhs.mir new file mode 100644 index 0000000..b145a6d --- /dev/null +++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-commute-int-const-lhs.mir @@ -0,0 +1,28 @@ +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4 +# RUN: llc -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner %s -o - \ +# RUN: --aarch64prelegalizercombiner-disable-rule=constant_fold_binop | FileCheck %s + +# `constant_fold_binop` is disabled to trigger the infinite loop in `commute_int_constant_to_rhs`. + +--- +name: add +tracksRegLiveness: true +body: | + bb.0: + liveins: $s0 + + ; CHECK-LABEL: name: add + ; CHECK: liveins: $s0 + ; CHECK-NEXT: {{ $}} + ; CHECK-NEXT: %c0:_(s32) = G_CONSTANT i32 1 + ; CHECK-NEXT: %c1:_(s32) = G_CONSTANT i32 2 + ; CHECK-NEXT: %add:_(s32) = G_ADD %c0, %c1 + ; CHECK-NEXT: $s0 = COPY %add(s32) + ; CHECK-NEXT: RET_ReallyLR + %c0:_(s32) = G_CONSTANT i32 1 + %c1:_(s32) = G_CONSTANT i32 2 + %add:_(s32) = G_ADD %c0, %c1 + $s0 = COPY %add + RET_ReallyLR + +... -- cgit v1.1