[llvm] de55850 - RegisterCoalescer: Correctly set valid lanes when keeping live out implicit defs
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 11 23:28:40 PDT 2023
Author: Matt Arsenault
Date: 2023-09-12T09:28:33+03:00
New Revision: de5585078e910d8b31369d027a0bea97e31664a2
URL: https://github.com/llvm/llvm-project/commit/de5585078e910d8b31369d027a0bea97e31664a2
DIFF: https://github.com/llvm/llvm-project/commit/de5585078e910d8b31369d027a0bea97e31664a2.diff
LOG: RegisterCoalescer: Correctly set valid lanes when keeping live out implicit defs
This fixes some verifier errors when live out implicit defs are
coalesced with identity copies. Fixes some reduced testcases from
issue #38788 but doesn't solve the original failure.
I was surprised this seems to obviate the special casing in
analyzeValue that's been there since the subregister liveness support
went in.
https://reviews.llvm.org/D158850
Added:
llvm/test/CodeGen/X86/pr38795-verifier-error-pr38788.mir
Modified:
llvm/lib/CodeGen/RegisterCoalescer.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 826fc916ec083ba..8532af141bf6c93 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -2445,6 +2445,15 @@ class JoinVals {
Val() = default;
bool isAnalyzed() const { return WriteLanes.any(); }
+
+ /// Mark this value as an IMPLICIT_DEF which must be kept as if it were an
+ /// ordinary value.
+ void mustKeepImplicitDef(const TargetRegisterInfo &TRI,
+ const MachineInstr &ImpDef) {
+ assert(ImpDef.isImplicitDef());
+ ErasableImplicitDef = false;
+ ValidLanes = TRI.getSubRegIndexLaneMask(ImpDef.getOperand(0).getSubReg());
+ }
};
/// One entry per value number in LI.
@@ -2786,13 +2795,15 @@ JoinVals::analyzeValue(unsigned ValNo, JoinVals &Other) {
//
// When it happens, treat that IMPLICIT_DEF as a normal value, and don't try
// to erase the IMPLICIT_DEF instruction.
- MachineBasicBlock *OtherMBB = Indexes->getMBBFromIndex(V.OtherVNI->def);
+ MachineInstr *OtherImpDef =
+ Indexes->getInstructionFromIndex(V.OtherVNI->def);
+ MachineBasicBlock *OtherMBB = OtherImpDef->getParent();
if (DefMI && DefMI->getParent() != OtherMBB) {
LLVM_DEBUG(dbgs() << "IMPLICIT_DEF defined at " << V.OtherVNI->def
<< " extends into "
<< printMBBReference(*DefMI->getParent())
<< ", keeping it.\n");
- OtherV.ErasableImplicitDef = false;
+ OtherV.mustKeepImplicitDef(*TRI, *OtherImpDef);
} else if (OtherMBB->hasEHPadSuccessor()) {
// If OtherV is defined in a basic block that has EH pad successors then
// we get the same problem not just if OtherV is live beyond its basic
@@ -2801,7 +2812,7 @@ JoinVals::analyzeValue(unsigned ValNo, JoinVals &Other) {
LLVM_DEBUG(
dbgs() << "IMPLICIT_DEF defined at " << V.OtherVNI->def
<< " may be live into EH pad successors, keeping it.\n");
- OtherV.ErasableImplicitDef = false;
+ OtherV.mustKeepImplicitDef(*TRI, *OtherImpDef);
} else {
// We deferred clearing these lanes in case we needed to save them
OtherV.ValidLanes &= ~OtherV.WriteLanes;
@@ -2957,20 +2968,6 @@ void JoinVals::computeAssignment(unsigned ValNo, JoinVals &Other) {
// The other value is going to be pruned if this join is successful.
assert(V.OtherVNI && "OtherVNI not assigned, can't prune");
Val &OtherV = Other.Vals[V.OtherVNI->id];
- // We cannot erase an IMPLICIT_DEF if we don't have valid values for all
- // its lanes.
- if (OtherV.ErasableImplicitDef &&
- TrackSubRegLiveness &&
- (OtherV.ValidLanes & ~V.ValidLanes).any()) {
- LLVM_DEBUG(dbgs() << "Cannot erase implicit_def with missing values\n");
-
- OtherV.ErasableImplicitDef = false;
- // The valid lanes written by the implicit_def were speculatively cleared
- // before, so make this more conservative. It may be better to track this,
- // I haven't found a testcase where it matters.
- OtherV.ValidLanes = LaneBitmask::getAll();
- }
-
OtherV.Pruned = true;
[[fallthrough]];
}
diff --git a/llvm/test/CodeGen/X86/pr38795-verifier-error-pr38788.mir b/llvm/test/CodeGen/X86/pr38795-verifier-error-pr38788.mir
new file mode 100644
index 000000000000000..83e5196717b882b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr38795-verifier-error-pr38788.mir
@@ -0,0 +1,169 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
+# RUN: llc -mtriple=i386-unknown-linux-gnu -verify-coalescing -run-pass=register-coalescer -o - %s | FileCheck %s
+
+# Make sure no verifier error is produced from coalescing the identity
+# copy in bb.3 with live out implicit_defs. The value is live out of
+# the block, and the incoming value is a phi def.
+
+---
+name: pr38795_verifier_error_reduced_1
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: pr38795_verifier_error_reduced_1
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $eax
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr32 = COPY $eax
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.2(0x40000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[DEF:%[0-9]+]]:gr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: JCC_1 %bb.3, 4, implicit undef $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[MOV32r0_:%[0-9]+]]:gr32 = MOV32r0 implicit-def dead $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3:
+ ; CHECK-NEXT: successors: %bb.5(0x40000000), %bb.4(0x40000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: JCC_1 %bb.5, 4, implicit undef $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.4:
+ ; CHECK-NEXT: successors: %bb.5(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr32 = COPY [[MOV32r0_]]
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.5:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gr32 = COPY [[COPY]]
+ ; CHECK-NEXT: JMP_1 %bb.1
+ bb.0:
+ liveins: $eax
+ %0:gr32 = COPY $eax
+
+ bb.1:
+ %1:gr32 = IMPLICIT_DEF
+ JCC_1 %bb.3, 4, implicit undef $eflags
+
+ bb.2:
+ %0:gr32 = MOV32r0 implicit-def dead $eflags
+
+ bb.3:
+ %0:gr32 = COPY %0
+ JCC_1 %bb.5, 4, implicit undef $eflags
+
+ bb.4:
+ %1:gr32 = COPY %0
+
+ bb.5:
+ %0:gr32 = COPY killed %1
+ JMP_1 %bb.1
+
+...
+
+# Check for "Instruction ending live segment doesn't read the
+# register" verifier error after coalescing
+# This still failed with a similar error after a preliminary fix was
+# attempted for pr38795_verifier_error_reduced_1
+
+---
+name: pr38795_verifier_error_reduced_2
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: pr38795_verifier_error_reduced_2
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[DEF:%[0-9]+]]:gr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.3(0x40000000), %bb.2(0x40000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[DEF1:%[0-9]+]]:gr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: JCC_1 %bb.3, 4, implicit killed undef $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: successors: %bb.9(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: JMP_1 %bb.9
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3:
+ ; CHECK-NEXT: successors: %bb.5(0x40000000), %bb.4(0x40000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: JCC_1 %bb.5, 5, implicit killed undef $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.4:
+ ; CHECK-NEXT: successors: %bb.6(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[MOV32r0_:%[0-9]+]]:gr32 = MOV32r0 implicit-def dead $eflags
+ ; CHECK-NEXT: JMP_1 %bb.6
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.5:
+ ; CHECK-NEXT: successors: %bb.9(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[DEF1:%[0-9]+]]:gr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: JMP_1 %bb.9
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.6:
+ ; CHECK-NEXT: successors: %bb.8(0x40000000), %bb.7(0x40000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: JCC_1 %bb.8, 4, implicit killed undef $eflags
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.7:
+ ; CHECK-NEXT: successors: %bb.8(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr32 = COPY [[DEF1]]
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.8:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gr32 = COPY [[COPY]]
+ ; CHECK-NEXT: JMP_1 %bb.1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.9:
+ ; CHECK-NEXT: successors: %bb.6(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: JMP_1 %bb.6
+ bb.0:
+ %0:gr32 = IMPLICIT_DEF
+
+ bb.1:
+ %1:gr32 = IMPLICIT_DEF
+ JCC_1 %bb.3, 4, implicit killed undef $eflags
+
+ bb.2:
+ %2:gr32 = COPY killed %0
+ JMP_1 %bb.9
+
+ bb.3:
+ JCC_1 %bb.5, 5, implicit killed undef $eflags
+
+ bb.4:
+ %3:gr32 = MOV32r0 implicit-def dead $eflags
+ JMP_1 %bb.6
+
+ bb.5:
+ %2:gr32 = IMPLICIT_DEF
+ JMP_1 %bb.9
+
+ bb.6:
+ JCC_1 %bb.8, 4, implicit killed undef $eflags
+
+ bb.7:
+ %1:gr32 = COPY killed %3
+
+ bb.8:
+ %4:gr32 = COPY killed %1
+ %0:gr32 = COPY killed %4
+ JMP_1 %bb.1
+
+ bb.9:
+ %3:gr32 = COPY killed %2
+ JMP_1 %bb.6
+
+...
More information about the llvm-commits
mailing list