From: David Stenberg Date: Wed, 23 Oct 2019 09:35:32 +0000 (+0200) Subject: [DebugInfo] Stop describing imms in TargetInstrInfo's describeLoadedValue() impl X-Git-Tag: llvmorg-11-init~5935 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=74a72e684849d00dbe5cc784cf05d20fd8873cdb;p=platform%2Fupstream%2Fllvm.git [DebugInfo] Stop describing imms in TargetInstrInfo's describeLoadedValue() impl Summary: The default implementation of the describeLoadedValue() hook uses the MoveImm property to determine if an instruction moves an immediate. If an instruction has that property the function returns the second operand, assuming that that is the immediate value the instruction moves. As far as I can tell, the MoveImm property does not imply that the second operand is the immediate value, nor that any other operand necessarily holds the immediate value; it just means that the instruction moves some immediate value. One example where the second operand is not the immediate is SystemZ's LZER instruction, which moves a zero immediate implicitly: $f0S = LZER. That case triggered an out-of-bound assertion when getting the operand. I have added a test case for that instruction. Another example is ARM's MVN instruction, which holds the logical bitwise NOT'd value of the immediate that is moved. For the following reproducer: extern void foo(int); int main() { foo(-11); } an incorrect call site value would be emitted: $ clang --target=arm foo.c -O1 -g -Xclang -femit-debug-entry-values \ -c -o - | ./build/bin/llvm-dwarfdump - | \ grep -A2 call_site_parameter 0x00000058: DW_TAG_GNU_call_site_parameter DW_AT_location (DW_OP_reg0 R0) DW_AT_GNU_call_site_value (DW_OP_lit10) Another example is the A2_combineii instruction on Hexagon which moves two immediates to a super-register: $d0 = A2_combineii 20, 10. Perhaps these are rare exceptions, and most MoveImm instructions hold the immediate in the second operand, but in my opinion the default implementation of the hook should only describe values that it can, by some contract, guarantee are safe to describe, rather than leaving it up to the targets to override the exceptions, as that can silently result in incorrect call site values. This patch adds X86's relevant move immediate instructions to the target's hook implementation, so this commit should be a NFC for that target. We need to do the same for ARM and AArch64. Reviewers: djtodoro, NikolaPrica, aprantl, vsk Reviewed By: vsk Subscribers: kristof.beyls, hiraditya, llvm-commits Tags: #debug-info, #llvm Differential Revision: https://reviews.llvm.org/D69109 --- diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp index 6cae3b8..d99b2b0 100644 --- a/llvm/lib/CodeGen/TargetInstrInfo.cpp +++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp @@ -1130,9 +1130,6 @@ TargetInstrInfo::describeLoadedValue(const MachineInstr &MI) const { if (isCopyInstr(MI, SrcRegOp, DestRegOp)) { Op = SrcRegOp; return ParamLoadedValue(*Op, Expr); - } else if (MI.isMoveImmediate()) { - Op = &MI.getOperand(1); - return ParamLoadedValue(*Op, Expr); } return None; diff --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp index c29029d..889ba28 100644 --- a/llvm/lib/Target/X86/X86InstrInfo.cpp +++ b/llvm/lib/Target/X86/X86InstrInfo.cpp @@ -7638,12 +7638,17 @@ X86InstrInfo::describeLoadedValue(const MachineInstr &MI) const { return ParamLoadedValue(*Op, Expr);; } + case X86::MOV32ri: + case X86::MOV64ri: + case X86::MOV64ri32: + return ParamLoadedValue(MI.getOperand(1), Expr); case X86::XOR32rr: { if (MI.getOperand(1).getReg() == MI.getOperand(2).getReg()) return ParamLoadedValue(MachineOperand::CreateImm(0), Expr); return None; } default: + assert(!MI.isMoveImmediate() && "Unexpected MoveImm instruction"); return TargetInstrInfo::describeLoadedValue(MI); } } diff --git a/llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir b/llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir new file mode 100644 index 0000000..8a4e8b5 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir @@ -0,0 +1,83 @@ +# RUN: llc -debug-entry-values -start-after=livedebugvalues -o - %s | FileCheck %s + +# This test would previously trigger an assertion when trying to describe the +# call site value for callee()'s float parameter. +# +# Please note that at the time of creating this test the SystemZ target did not +# support call site information, so the "callSites" array has been manually +# added. For now, verify that we don't crash, and that no (invalid) call site +# parameter entry is emitted, but later on when call site information is added +# to the target the LZER instruction should be properly described. + +# CHECK-NOT: DW_TAG_GNU_call_site_parameter + +# Based on the following C reproducer: +# +# extern void callee(float); +# int caller() { +# callee(0); +# return 0; +# } + +--- | + target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64" + target triple = "systemz" + + ; Function Attrs: nounwind + define signext i32 @caller() !dbg !12 { + entry: + tail call void @callee(float 0.000000e+00), !dbg !16 + ret i32 0, !dbg !17 + } + + declare !dbg !4 void @callee(float) + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!8, !9, !10} + !llvm.ident = !{!11} + + !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None) + !1 = !DIFile(filename: "systemz-lzer.c", directory: "/") + !2 = !{} + !3 = !{!4} + !4 = !DISubprogram(name: "callee", scope: !1, file: !1, line: 1, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2) + !5 = !DISubroutineType(types: !6) + !6 = !{null, !7} + !7 = !DIBasicType(name: "float", size: 32, encoding: DW_ATE_float) + !8 = !{i32 2, !"Dwarf Version", i32 4} + !9 = !{i32 2, !"Debug Info Version", i32 3} + !10 = !{i32 1, !"wchar_size", i32 4} + !11 = !{!"clang version 10.0.0"} + !12 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line: 2, type: !13, scopeLine: 2, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2) + !13 = !DISubroutineType(types: !14) + !14 = !{!15} + !15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !16 = !DILocation(line: 3, scope: !12) + !17 = !DILocation(line: 4, scope: !12) + +... +--- +name: caller +tracksRegLiveness: true +callSites: + - { bb: 0, offset: 9, fwdArgRegs: + - { arg: 0, reg: '$f0s' } } +body: | + bb.0.entry: + liveins: $r11d, $r15d, $r14d + + STMG killed $r11d, killed $r15d, $r15d, 88, implicit killed $r14d + CFI_INSTRUCTION offset $r11d, -72 + CFI_INSTRUCTION offset $r14d, -48 + CFI_INSTRUCTION offset $r15d, -40 + $r15d = AGHI $r15d, -160, implicit-def dead $cc + CFI_INSTRUCTION def_cfa_offset 320 + $r11d = LGR $r15d + CFI_INSTRUCTION def_cfa_register $r11d + renamable $f0s = LZER + CallBRASL @callee, $f0s, csr_systemz, implicit-def dead $r14d, implicit-def dead $cc, implicit $fpc, debug-location !16 + $r2d = LGHI 0, debug-location !17 + $r11d, $r15d = LMG $r11d, 248, implicit-def $r14d, debug-location !17 + Return implicit killed $r2d, debug-location !17 + +... diff --git a/llvm/test/DebugInfo/MIR/SystemZ/lit.local.cfg b/llvm/test/DebugInfo/MIR/SystemZ/lit.local.cfg new file mode 100644 index 0000000..2f3cf7d --- /dev/null +++ b/llvm/test/DebugInfo/MIR/SystemZ/lit.local.cfg @@ -0,0 +1,2 @@ +if not 'SystemZ' in config.root.targets: + config.unsupported = True