[llvm] 6f6298e - [GlobalISel] Fix D144336 in a different way, by choosing operands from the first of the div/rem insts.
Amara Emerson via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 9 15:06:14 PDT 2023
Author: Amara Emerson
Date: 2023-06-09T15:06:06-07:00
New Revision: 6f6298e5b3489df6cb13dd3de8d77af21d03cf0b
URL: https://github.com/llvm/llvm-project/commit/6f6298e5b3489df6cb13dd3de8d77af21d03cf0b
DIFF: https://github.com/llvm/llvm-project/commit/6f6298e5b3489df6cb13dd3de8d77af21d03cf0b.diff
LOG: [GlobalISel] Fix D144336 in a different way, by choosing operands from the first of the div/rem insts.
Differential Revision: https://reviews.llvm.org/D144336
Added:
Modified:
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 94a756089f98a..f47664e310625 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -1113,7 +1113,6 @@ void CombinerHelper::applyCombineIndexedLoadStore(
bool CombinerHelper::matchCombineDivRem(MachineInstr &MI,
MachineInstr *&OtherMI) {
- OtherMI = nullptr;
unsigned Opcode = MI.getOpcode();
bool IsDiv, IsSigned;
@@ -1168,44 +1167,11 @@ bool CombinerHelper::matchCombineDivRem(MachineInstr &MI,
matchEqualDefs(MI.getOperand(2), UseMI.getOperand(2)) &&
matchEqualDefs(MI.getOperand(1), UseMI.getOperand(1))) {
OtherMI = &UseMI;
- break;
+ return true;
}
}
- if (!OtherMI)
- return false;
-
- // We may have a situation like this:
- // %4:_(s32) = G_SEXT %2:_(s1)
- // %5:_(s32) = G_SEXT %2:_(s1)
- // %6:_(s32) = G_UDIV %4:_, %5:_
- // %8:_(s32) = G_SEXT %2:_(s1)
- // %9:_(s32) = G_UREM %5:_, %8:_
- // and choosing the insertion point as the G_UDIV will cause it to use %8
- // before the def. We check here if any of the operands of the later
- // instruction (i.e. one of DIV/REM that is the second in the block) are
- // dominated by the first instruction. In this case we check if %8 is
- // dominated by the G_UDIV and bail out if so.
- SmallSet<Register, 2> RegsToCheck;
- MachineInstr *First, *Second;
- if (dominates(MI, *OtherMI)) {
- First = &MI;
- Second = OtherMI;
- } else {
- First = OtherMI;
- Second = &MI;
- }
- RegsToCheck.insert(Second->getOperand(1).getReg());
- RegsToCheck.insert(Second->getOperand(2).getReg());
- for (MachineBasicBlock::iterator II = std::next(First->getIterator());
- II != Second->getIterator(); ++II) {
- for (auto &MO : II->operands()) {
- if (MO.isReg() && MO.isDef() && RegsToCheck.count(MO.getReg()) &&
- dominates(*First, *II))
- return false;
- }
- }
- return true;
+ return false;
}
void CombinerHelper::applyCombineDivRem(MachineInstr &MI,
@@ -1226,16 +1192,22 @@ void CombinerHelper::applyCombineDivRem(MachineInstr &MI,
Opcode == TargetOpcode::G_SDIV || Opcode == TargetOpcode::G_SREM;
// Check which instruction is first in the block so we don't break def-use
- // deps by "moving" the instruction incorrectly.
- if (dominates(MI, *OtherMI))
+ // deps by "moving" the instruction incorrectly. Also keep track of which
+ // instruction is first so we pick it's operands, avoiding use-before-def
+ // bugs.
+ MachineInstr *FirstInst;
+ if (dominates(MI, *OtherMI)) {
Builder.setInstrAndDebugLoc(MI);
- else
+ FirstInst = &MI;
+ } else {
Builder.setInstrAndDebugLoc(*OtherMI);
+ FirstInst = OtherMI;
+ }
Builder.buildInstr(IsSigned ? TargetOpcode::G_SDIVREM
: TargetOpcode::G_UDIVREM,
{DestDivReg, DestRemReg},
- {MI.getOperand(1).getReg(), MI.getOperand(2).getReg()});
+ { FirstInst->getOperand(1), FirstInst->getOperand(2) });
MI.eraseFromParent();
OtherMI->eraseFromParent();
}
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir
index 4911fdaf76d4c..abccb18502494 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/prelegalizer-combiner-divrem-insertpt-conflict.mir
@@ -1,7 +1,6 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - | FileCheck %s
-# Check that we don't combine to G_UDIVREM if it would cause a use-before-def
---
name: test
alignment: 4
@@ -12,13 +11,11 @@ body: |
; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
- ; CHECK-NEXT: [[UDIV:%[0-9]+]]:_(s32) = G_UDIV [[SEXT]], [[SEXT1]]
- ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
- ; CHECK-NEXT: [[UREM:%[0-9]+]]:_(s32) = G_UREM [[SEXT1]], [[SEXT2]]
- ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UREM]](s32)
- ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIV]](s32)
- ; CHECK-NEXT: [[SEXT3:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
- ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT3]]
+ ; CHECK-NEXT: [[UDIVREM:%[0-9]+]]:_(s32), [[UDIVREM1:%[0-9]+]]:_ = G_UDIVREM [[SEXT]], [[SEXT1]]
+ ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM1]](s32)
+ ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM]](s32)
+ ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
+ ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT2]]
; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
; CHECK-NEXT: $w0 = COPY [[TRUNC1]](s32)
; CHECK-NEXT: RET_ReallyLR implicit $w0
@@ -53,48 +50,9 @@ body: |
; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
- ; CHECK-NEXT: [[UREM:%[0-9]+]]:_(s32) = G_UREM [[SEXT]], [[SEXT1]]
- ; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
- ; CHECK-NEXT: [[UDIV:%[0-9]+]]:_(s32) = G_UDIV [[SEXT1]], [[SEXT2]]
- ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIV]](s32)
- ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UREM]](s32)
- ; CHECK-NEXT: [[SEXT3:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
- ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT3]]
- ; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
- ; CHECK-NEXT: $w0 = COPY [[TRUNC1]](s32)
- ; CHECK-NEXT: RET_ReallyLR implicit $w0
- %0:_(s16) = G_CONSTANT i16 0
- %2:_(s1) = G_CONSTANT i1 true
- %1:_(s1) = G_ICMP intpred(sge), %0(s16), %0
- %3:_(s8) = G_SEXT %2(s1)
- %4:_(s32) = G_SEXT %3(s8)
- %5:_(s32) = G_SEXT %1(s1)
- %6:_(s32) = G_UREM %4, %5
- %7:_(s32) = COPY %5(s32)
- %8:_(s32) = G_SEXT %2(s1)
- %9:_(s32) = G_UDIV %7, %8
- %10:_(s8) = G_TRUNC %9(s32)
- %11:_(s64) = G_ZEXT %6(s32)
- %12:_(s64) = G_SEXT %10(s8)
- %13:_(s64) = G_OR %11, %12
- %14:_(s32) = G_TRUNC %13(s64)
- $w0 = COPY %14(s32)
- RET_ReallyLR implicit $w0
-
-...
----
-name: ok_before_first
-alignment: 4
-tracksRegLiveness: true
-body: |
- bb.1:
- ; CHECK-LABEL: name: ok_before_first
- ; CHECK: [[C:%[0-9]+]]:_(s1) = G_CONSTANT i1 true
- ; CHECK-NEXT: [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
- ; CHECK-NEXT: [[SEXT1:%[0-9]+]]:_(s32) = G_SEXT [[C]](s1)
; CHECK-NEXT: [[UDIVREM:%[0-9]+]]:_(s32), [[UDIVREM1:%[0-9]+]]:_ = G_UDIVREM [[SEXT]], [[SEXT1]]
- ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM1]](s32)
- ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM]](s32)
+ ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[UDIVREM]](s32)
+ ; CHECK-NEXT: [[ZEXT:%[0-9]+]]:_(s64) = G_ZEXT [[UDIVREM1]](s32)
; CHECK-NEXT: [[SEXT2:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s8)
; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[ZEXT]], [[SEXT2]]
; CHECK-NEXT: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[OR]](s64)
@@ -106,10 +64,10 @@ body: |
%3:_(s8) = G_SEXT %2(s1)
%4:_(s32) = G_SEXT %3(s8)
%5:_(s32) = G_SEXT %1(s1)
- %8:_(s32) = G_SEXT %2(s1)
- %6:_(s32) = G_UDIV %4, %5
+ %6:_(s32) = G_UREM %4, %5
%7:_(s32) = COPY %5(s32)
- %9:_(s32) = G_UREM %7, %8
+ %8:_(s32) = G_SEXT %2(s1)
+ %9:_(s32) = G_UDIV %7, %8
%10:_(s8) = G_TRUNC %9(s32)
%11:_(s64) = G_ZEXT %6(s32)
%12:_(s64) = G_SEXT %10(s8)
More information about the llvm-commits
mailing list