[llvm] cf07277 - [X86] Fix the LEA optimization pass
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 18 17:50:26 PDT 2022
Author: Kazu Hirata
Date: 2022-09-18T17:50:17-07:00
New Revision: cf07277fb47276ba1c3cd661f096098572a6cfb3
URL: https://github.com/llvm/llvm-project/commit/cf07277fb47276ba1c3cd661f096098572a6cfb3
DIFF: https://github.com/llvm/llvm-project/commit/cf07277fb47276ba1c3cd661f096098572a6cfb3.diff
LOG: [X86] Fix the LEA optimization pass
The LEA optimization pass visits each basic block of a given machine
function. In each basic block, for each pair of LEAs that differ only
in their displacement fields, we replace all uses of the second LEA
with the first LEA while adjusting the displacement.
Now, without this patch, after all the replacements are made, the
following assert triggers:
assert(MRI->use_empty(LastVReg) &&
"The LEA's def register must have no uses");
The replacement loop uses:
for (MachineOperand &MO :
llvm::make_early_inc_range(MRI->use_operands(LastVReg))) {
which is equivalent to:
for (auto UI = MRI->use_begin(LastVReg), UE = MRI->use_end();
UI != UE;) {
MachineOperand &MO = *UI++; // <-- Look!
That is, immediately after the post increment, make_early_inc_range
already has the iterator for the next iteration in its mind.
The problem is that in one iteration of the loop, we could replace two
uses in a debug instruction like:
DBG_VALUE_LIST !"r", !DIExpression(DW_OP_LLVM_arg, 0), %0:gr64, %0:gr64, ...
So, the iterator for the next iteration becomes invalid. We end up
traversing a garbage use list from that point on. In turn, we don't
get to visit remaining uses.
The patch fixes the problem by switching to a "draining" while loop:
while (!MRI->use_empty(LastVReg)) {
MachineOperand &MO = *MRI->use_begin(LastVReg);
MachineInstr &MI = *MO.getParent();
The credit goes to Simon Pilgrim for reducing the test case.
Fixes https://github.com/llvm/llvm-project/issues/57673
Differential Revision: https://reviews.llvm.org/D133631
Added:
llvm/test/CodeGen/X86/pr57673.ll
llvm/test/CodeGen/X86/pr57673.mir
Modified:
llvm/lib/Target/X86/X86OptimizeLEAs.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/X86/X86OptimizeLEAs.cpp b/llvm/lib/Target/X86/X86OptimizeLEAs.cpp
index 780a0223b3409..40ef69f3f3f38 100644
--- a/llvm/lib/Target/X86/X86OptimizeLEAs.cpp
+++ b/llvm/lib/Target/X86/X86OptimizeLEAs.cpp
@@ -653,8 +653,12 @@ bool X86OptimizeLEAPass::removeRedundantLEAs(MemOpMap &LEAs) {
// isReplaceable function.
Register FirstVReg = First.getOperand(0).getReg();
Register LastVReg = Last.getOperand(0).getReg();
- for (MachineOperand &MO :
- llvm::make_early_inc_range(MRI->use_operands(LastVReg))) {
+ // We use MRI->use_empty here instead of the combination of
+ // llvm::make_early_inc_range and MRI->use_operands because we could
+ // replace two or more uses in a debug instruction in one iteration, and
+ // that would deeply confuse llvm::make_early_inc_range.
+ while (!MRI->use_empty(LastVReg)) {
+ MachineOperand &MO = *MRI->use_begin(LastVReg);
MachineInstr &MI = *MO.getParent();
if (MI.isDebugValue()) {
diff --git a/llvm/test/CodeGen/X86/pr57673.ll b/llvm/test/CodeGen/X86/pr57673.ll
new file mode 100644
index 0000000000000..91fd3af4fceb5
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr57673.ll
@@ -0,0 +1,70 @@
+; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=x86-64 -stop-after=x86-optimize-LEAs < %s | FileCheck %s
+
+; The LEA optimization pass used to crash on this testcase.
+
+; This test case used to trigger:
+;
+; assert(MRI->use_empty(LastVReg) &&
+; "The LEA's def register must have no uses");
+
+; CHECK: LEA64r
+; CHECK-NOT: LEA64r
+; CHECK: DBG_VALUE_LIST
+
+target triple = "x86_64-unknown-linux-gnu"
+
+%t10 = type { i8*, [32 x i8] }
+
+define void @foo() {
+bb_entry:
+ %tmp11 = alloca [0 x [0 x i32]], i32 0, align 4
+ %i = alloca %t10, align 8
+ %i1 = alloca %t10, align 8
+ %tmp1.sub = getelementptr inbounds [0 x [0 x i32]], [0 x [0 x i32]]* %tmp11, i64 0, i64 0, i64 0
+ %i2 = bitcast [0 x [0 x i32]]* %tmp11 to i8*
+ br label %bb_8
+
+bb_8: ; preds = %bb_last, %bb_entry
+ br i1 undef, label %bb_last, label %bb_mid
+
+bb_mid: ; preds = %bb_8
+ %i3 = bitcast %t10* %i1 to i8*
+ %i4 = getelementptr inbounds %t10, %t10* %i1, i64 0, i32 1, i64 32
+ %i5 = bitcast %t10* %i to i8*
+ %i6 = getelementptr inbounds %t10, %t10* %i, i64 0, i32 1, i64 32
+ call void @llvm.lifetime.start.p0i8(i64 0, i8* nonnull %i3)
+ %v21 = call i64 @llvm.ctlz.i64(i64 undef, i1 false)
+ call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef nonnull dereferenceable(16) null, i8* noundef nonnull align 8 dereferenceable(16) %i4, i64 16, i1 false)
+ call void @llvm.dbg.value(metadata !DIArgList(i8* %i4, i8* %i4), metadata !4, metadata !DIExpression(DW_OP_LLVM_arg, 0)), !dbg !9
+ call void @llvm.lifetime.end.p0i8(i64 0, i8* nonnull %i3)
+ call void @llvm.lifetime.start.p0i8(i64 0, i8* nonnull %i5)
+ call void @llvm.memcpy.p0i8.p0i8.i64(i8* noundef nonnull dereferenceable(16) null, i8* noundef nonnull align 8 dereferenceable(16) %i6, i64 16, i1 false)
+ call void @llvm.lifetime.end.p0i8(i64 0, i8* nonnull %i5)
+ br label %bb_last
+
+bb_last: ; preds = %bb_mid, %bb_8
+ call void @llvm.lifetime.start.p0i8(i64 0, i8* nonnull %i2)
+ call void undef(i32* null, i32* null, i32* null, i32 0, i32* nonnull %tmp1.sub)
+ call void @llvm.lifetime.end.p0i8(i64 0, i8* nonnull %i2)
+ br label %bb_8
+}
+
+declare i64 @llvm.ctlz.i64(i64, i1 immarg)
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+!1 = !DIFile(filename: "n", directory: "/proc/self/cwd", checksumkind: CSK_MD5, checksum: "e588179fedd8fcdfada963f2434cb950")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = !{!"function_entry_count", i64 2423}
+!4 = !DILocalVariable(name: "r", scope: !5, file: !6, line: 93)
+!5 = distinct !DISubprogram(name: "c", scope: !7, file: !6, line: 92, spFlags: DISPFlagDefinition, unit: !0)
+!6 = !DIFile(filename: "a", directory: "/proc/self/cwd")
+!7 = !DINamespace(name: "u", scope: !8)
+!8 = !DINamespace(name: "s", scope: null)
+!9 = !DILocation(line: 0, scope: !5)
diff --git a/llvm/test/CodeGen/X86/pr57673.mir b/llvm/test/CodeGen/X86/pr57673.mir
new file mode 100644
index 0000000000000..c7557da28c85f
--- /dev/null
+++ b/llvm/test/CodeGen/X86/pr57673.mir
@@ -0,0 +1,35 @@
+# RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=x86-64 -run-pass=x86-optimize-LEAs -o - %s | FileCheck %s
+
+# This test case used to trigger the assertion in the LEA optimization pass:
+#
+# assert(MRI->use_empty(LastVReg) &&
+# "The LEA's def register must have no uses");
+
+# CHECK: LEA64r
+# CHECK-NOT: LEA64r
+# CHECK: DBG_VALUE_LIST
+
+--- |
+ define void @foo() {
+ ret void
+ }
+
+ !0 = !DIFile(filename: "a", directory: "/proc/self/cwd")
+ !1 = distinct !DISubprogram(name: "c", scope: null, file: !0, line: 3)
+ !2 = !DILocalVariable(name: "r", scope: !1, file: !0, line: 4)
+ !3 = !DILocation(line: 5, scope: !1)
+...
+---
+name: foo
+alignment: 16
+tracksRegLiveness: true
+stack:
+ - { id: 0, size: 40, alignment: 8 }
+body: |
+ bb.0:
+ %0:gr64 = LEA64r %stack.0, 1, $noreg, 0, $noreg
+ %1:gr64 = LEA64r %stack.0, 1, $noreg, 40, $noreg
+ DBG_VALUE_LIST !2, !DIExpression(DW_OP_LLVM_arg, 0), %1:gr64, %1:gr64, debug-location !3
+ %2:vr128 = MOVUPSrm %1:gr64, 1, $noreg, 0, $noreg
+ RET64
+...
More information about the llvm-commits
mailing list