[llvm] 241f3e3 - [DebugInfo][InstrRef] Fix a broken substitution method, add test coverage
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 20 03:45:25 PDT 2021
Author: Jeremy Morse
Date: 2021-07-20T11:45:13+01:00
New Revision: 241f3e386cd2ce176ac32a957732664ad69b6144
URL: https://github.com/llvm/llvm-project/commit/241f3e386cd2ce176ac32a957732664ad69b6144
DIFF: https://github.com/llvm/llvm-project/commit/241f3e386cd2ce176ac32a957732664ad69b6144.diff
LOG: [DebugInfo][InstrRef] Fix a broken substitution method, add test coverage
This patch fixes a clearly-broken function that I absent-mindedly bodged
many months ago.
Over in D85749 I landed the substituteDebugValuesForInst, that creates
substitution records for all the def operands from one debug-labelled
instruction to the new one. Unfortunately it would crash if the two
instructions had different numbers of operands; I tried to fix this in
537f0fbe82 by adding a "max operand" parameter to the method, but then
didn't actually change the loop bound to take account of this. It passed
all the tests because.... well there wasn't any real test coverage of this
method.
This patch fixes up the loop to be bounded by the MaxOperand bound; and
adds test coverage for the x86-fixup-LEAs calls to this method, so that
it's actually tested.
Differential Revision: https://reviews.llvm.org/D105820
Added:
llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir
llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup.mir
Modified:
llvm/lib/CodeGen/MachineFunction.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index d26f581539700..72e58700c75f2 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -990,7 +990,7 @@ void MachineFunction::substituteDebugValuesForInst(const MachineInstr &Old,
// MIR output.
// Examine all the operands, or the first N specified by the caller.
MaxOperand = std::min(MaxOperand, Old.getNumOperands());
- for (unsigned int I = 0; I < Old.getNumOperands(); ++I) {
+ for (unsigned int I = 0; I < MaxOperand; ++I) {
const auto &OldMO = Old.getOperand(I);
auto &NewMO = New.getOperand(I);
(void)NewMO;
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir
new file mode 100644
index 0000000000000..2ffd6b5e49b52
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup-2.mir
@@ -0,0 +1,62 @@
+# RUN: llc -run-pass x86-fixup-LEAs -mtriple=i386 -verify-machineinstrs -mcpu=corei7-avx -o - %s | FileCheck %s
+#
+# Test that several LEA => ADD transforms get substitutions applied to them,
+# for corner cases that we can only hit with -mtriple=i386.
+---
+name: test2add_32
+# CHECK: name: test2add_32
+alignment: 16
+tracksRegLiveness: true
+liveins:
+ - { reg: '$eax' }
+ - { reg: '$ebp' }
+# CHECK: debugValueSubstitutions:
+# CHECK-NAME: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
+body: |
+ bb.0:
+ liveins: $eax, $ebp
+
+ ; CHECK: $eax = ADD32ri8 {{.*}} debug-instr-number 2
+ $eax = LEA32r killed $eax, 1, killed $ebp, -5, $noreg, debug-instr-number 1
+ RETQ $eax
+
+...
+---
+name: test1mov1add_ebp_32
+# CHECK-LABEL: name: test1mov1add_ebp_32
+alignment: 16
+tracksRegLiveness: true
+liveins:
+ - { reg: '$eax' }
+ - { reg: '$ebx' }
+ - { reg: '$ebp' }
+# CHECK: debugValueSubstitutions:
+# CHECK-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
+body: |
+ bb.0:
+ liveins: $eax, $ebp, $ebx
+
+ ; CHECK: $ebx = ADD32rr {{.*}} debug-instr-number 2
+ $ebx = LEA32r killed $ebp, 1, $ebp, 0, $noreg, debug-instr-number 1
+ RETQ $ebx
+
+...
+---
+name: testleaadd_ebp_index_32
+# CHECK-LABEL: name: testleaadd_ebp_index_32
+alignment: 16
+tracksRegLiveness: true
+liveins:
+ - { reg: '$ebx' }
+ - { reg: '$ebp' }
+# CHECK: debugValueSubstitutions:
+# CHECK-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
+body: |
+ bb.0:
+ liveins: $eax, $ebp, $ebx
+
+ ; CHECK: $ebx = ADD32rr {{.*}} debug-instr-number 2
+ $ebx = LEA32r $ebp, 1, $ebp, 5, $noreg, debug-instr-number 1
+ RETQ $ebx
+
+...
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup.mir
new file mode 100644
index 0000000000000..f8b7967e88b6b
--- /dev/null
+++ b/llvm/test/DebugInfo/MIR/InstrRef/x86-lea-fixup.mir
@@ -0,0 +1,77 @@
+# RUN: llc -run-pass x86-fixup-LEAs -mtriple=x86_64-gnu-unknown -verify-machineinstrs -mcpu=corei7-avx -o - %s | FileCheck %s --check-prefix=COREI7
+# RUN: llc -run-pass x86-fixup-LEAs -mtriple=x86_64-gnu-unknown -verify-machineinstrs -mcpu=haswell -o - %s | FileCheck %s --check-prefix=HASWELL
+# RUN: llc -run-pass x86-fixup-LEAs -mtriple=x86_64-unknown-unknown -verify-machineinstrs -mcpu=atom -o - %s | FileCheck %s --check-prefix=ATOM
+#
+# Test several LEA <=> ADD transformations that the fixup-leas pass performs,
+# and check that any debug-instr-number attached to the original instruction
+# is substituted onto the new instruction.
+# Some are only reachable under specific CPU modes it seems -- each function
+# in this file is only tested by one prefix / CPU mode. Some i386 specific
+# behaviours are in the -2 flavour of this file.
+---
+# COREI7-LABE: name: pr43758
+name: pr43758
+alignment: 16
+tracksRegLiveness: true
+liveins:
+ - { reg: '$rax' }
+ - { reg: '$rbp' }
+# COREI7: debugValueSubstitutions:
+# COREI7-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
+body: |
+ bb.0:
+ liveins: $rax, $rbp
+
+ $ebp = LEA64_32r killed $rbp, 1, killed $rax, 0, $noreg, debug-instr-number 1
+ ; COREI7: ADD32rr {{.*}} debug-instr-number 2
+ RETQ $ebp
+
+...
+---
+name: test_mul_spec
+# HASWELL-LABEL: name: test_mul_spec
+alignment: 16
+tracksRegLiveness: true
+liveins:
+ - { reg: '$edi' }
+frameInfo:
+ maxAlignment: 1
+ maxCallFrameSize: 0
+machineFunctionInfo: {}
+# HASWELL: debugValueSubstitutions:
+# HASWELL-NEXT: - { srcinst: 1, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
+# HASWELL-NEXT: - { srcinst: 2, srcop: 0, dstinst: 4, dstop: 0, subreg: 0 }
+body: |
+ bb.0:
+ liveins: $edi
+
+ renamable $edi = KILL $edi, implicit-def $rdi
+ renamable $ecx = nsw LEA64_32r renamable $rdi, 8, renamable $rdi, 42, $noreg, debug-instr-number 1
+ ; HASWELL: ADD32ri8 {{.*}} debug-instr-number 3
+ renamable $eax = nsw LEA64_32r killed renamable $rdi, 4, renamable $rdi, 2, $noreg, debug-instr-number 2
+ ; HASWELL: ADD32ri8 {{.*}} debug-instr-number 4
+ renamable $eax = nsw IMUL32rr killed renamable $eax, killed renamable $ecx, implicit-def dead $eflags
+ RETQ $eax
+
+...
+---
+name: testthree
+# ATOM-LABEL: name: testthree
+alignment: 16
+tracksRegLiveness: true
+frameInfo:
+ maxAlignment: 1
+ maxCallFrameSize: 0
+machineFunctionInfo: {}
+# ATOM: debugValueSubstitutions:
+# ATOM-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
+body: |
+ bb.0.entry:
+ renamable $ecx = MOV32ri 0
+ renamable $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags
+ renamable $ecx = nsw ADD32rr renamable $ecx, renamable $eax, implicit-def dead $eflags, implicit killed $rax, implicit killed $rcx, implicit-def $rcx, debug-instr-number 1
+ ; ATOM: LEA64_32r {{.*}} debug-instr-number 2
+ renamable $eax = MOV32rm killed renamable $rcx, 1, $noreg, 0, $noreg :: (load (s32) from `i32 *undef`)
+ RETQ $eax
+
+...
More information about the llvm-commits
mailing list