[PATCH] D80691: Proposed fix for PR46114
Qirun Zhang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun May 31 11:10:19 PDT 2020
helloqirun updated this revision to Diff 267509.
helloqirun added a comment.
I have added the test case and made a minor change. The `RemoveRedundantDbgInstrs` call will always keep the second `dbg.value`. If we do RAUW first, the corresponding value will be replaced -- that causes PR46114. With this patch, we set it as undef. If I missed anything, please let me know. I think @Orlando's suggestion on merging the locations will be a good improvement.
Based on @Orlando's comment. I am attaching the dwarfdump as follows.
- The version before the adoption of `RemoveRedundantDbgInstrs`.
entry:
%0 = load i32, i32* @a, align 4, !dbg !26, !tbaa !27
call void @llvm.dbg.value(metadata i32 %0, metadata !21, metadata !DIExpression()), !dbg !31
call void @llvm.dbg.value(metadata i32 0, metadata !22, metadata !DIExpression()), !dbg !31
call void @llvm.dbg.value(metadata i32 %0, metadata !22, metadata !DIExpression()), !dbg !31
%1 = load i8, i8* @c, align 1, !dbg !32, !tbaa !33
%conv = sext i8 %1 to i32, !dbg !32
%sub = sub nsw i32 %0, %conv, !dbg !34
%conv1 = trunc i32 %sub to i16, !dbg !35
$ llvm-dwarfdump a.out -name l_52
a.out: file format ELF64-x86-64
0x00000097: DW_TAG_variable
DW_AT_location (0x00000000:
[0x0000000000400486, 0x0000000000400499): DW_OP_consts +0, DW_OP_stack_value)
DW_AT_name ("l_52")
DW_AT_decl_file ("/home/absozero/bisect/abc.c")
DW_AT_decl_line (5)
DW_AT_type (0x0000003f "int")
We can see two `dbg.value`s. But it's still incorrect as the value of l_52 will always be 0.
- The version before applying the patch.
entry:
%0 = load i32, i32* @a, align 4, !dbg !26, !tbaa !27
call void @llvm.dbg.value(metadata i32 %0, metadata !21, metadata !DIExpression()), !dbg !31
call void @llvm.dbg.value(metadata i32 %0, metadata !22, metadata !DIExpression()), !dbg !31
%1 = load i8, i8* @c, align 1, !dbg !32, !tbaa !33
%conv = sext i8 %1 to i32, !dbg !32
$ llvm-dwarfdump a.out -name l_52
a.out: file format ELF64-x86-64
0x00000097: DW_TAG_variable
DW_AT_location (0x00000000:
[0x0000000000400486, 0x000000000040048f): DW_OP_reg0 RAX)
DW_AT_name ("l_52")
DW_AT_decl_file ("/home/absozero/bisect/abc.c")
DW_AT_decl_line (5)
DW_AT_type (0x0000003f "int")
`RemoveRedundantDbgInstrs` eliminates the first `dbg.value` and lldb gives a wrong value (PR46144).
- Version after applying the patch
entry:
%0 = load i32, i32* @a, align 4, !dbg !26, !tbaa !27
call void @llvm.dbg.value(metadata i32 %0, metadata !21, metadata !DIExpression()), !dbg !31
call void @llvm.dbg.value(metadata i32 undef, metadata !22, metadata !DIExpression()), !dbg !31
%1 = load i8, i8* @c, align 1, !dbg !32, !tbaa !33
%conv = sext i8 %1 to i32, !dbg !32
$ llvm-dwarfdump a.out -name l_52
a.out: file format ELF64-x86-64
0x00000097: DW_TAG_variable
DW_AT_location (0x00000000:
[0x0000000000400486, 0x000000000040048f): DW_OP_reg0 RAX)
DW_AT_name ("l_52")
DW_AT_decl_file ("/home/absozero/bisect/abc.c")
DW_AT_decl_line (5)
DW_AT_type (0x0000003f "int")
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80691/new/
https://reviews.llvm.org/D80691
Files:
llvm/lib/Transforms/Scalar/EarlyCSE.cpp
llvm/test/DebugInfo/X86/earlycse-load-marks-undef.ll
Index: llvm/test/DebugInfo/X86/earlycse-load-marks-undef.ll
===================================================================
--- /dev/null
+++ llvm/test/DebugInfo/X86/earlycse-load-marks-undef.ll
@@ -0,0 +1,65 @@
+; RUN: opt -mtriple=x86_64-- %s -S --early-cse -o - | FileCheck %s
+; Ensure that the second dbg.value is undef as RemoveRedundantDbgInstrs
+; will remove the first dbg.value.
+; Bugzilla #46114
+
+ at a = dso_local global i32 1, align 4, !dbg !0
+ at c = dso_local global i8 0, align 1, !dbg !9
+ at b = dso_local global i16 0, align 2, !dbg !6
+
+define dso_local void @main() #0 !dbg !15 {
+entry:
+ %0 = load i32, i32* @a, align 4
+ call void @llvm.dbg.value(metadata i32 0, metadata !20, metadata !DIExpression()), !dbg !24
+ %1 = load i32, i32* @a, align 4, !dbg !25, !tbaa !26
+; CHECK: call void @llvm.dbg.value(metadata i32 undef
+ call void @llvm.dbg.value(metadata i32 %1, metadata !20, metadata !DIExpression()), !dbg !24
+ %sub = sub nsw i32 %1, 0
+ %conv1 = trunc i32 %sub to i16
+ store i16 %conv1, i16* @b, align 2, !dbg !30, !tbaa !31
+ %tobool = icmp ne i32 %0, 0
+ unreachable
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!13, !14}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !12, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git 20c9bb44ec1a4a795215ff6964d264219f9b05f2)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "earlycse-load-marks-undef.ll", directory: "/temp/bz46114")
+!4 = !{}
+!5 = !{!0, !6, !9}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
+!8 = !DIBasicType(name: "short", size: 16, encoding: DW_ATE_signed)
+!9 = !DIGlobalVariableExpression(var: !10, expr: !DIExpression())
+!10 = distinct !DIGlobalVariable(name: "c", scope: !2, file: !3, line: 3, type: !11, isLocal: false, isDefinition: true)
+!11 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!13 = !{i32 2, !"Debug Info Version", i32 3}
+!14 = !{i32 1, !"wchar_size", i32 4}
+!15 = distinct !DISubprogram(name: "main", scope: !3, file: !3, line: 4, type: !16, scopeLine: 4, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !18)
+!16 = !DISubroutineType(types: !17)
+!17 = !{!12}
+!18 = !{!19, !20, !21}
+!19 = !DILocalVariable(name: "d", scope: !15, file: !3, line: 5, type: !12)
+!20 = !DILocalVariable(name: "l_52", scope: !15, file: !3, line: 5, type: !12)
+!21 = !DILocalVariable(name: "e", scope: !22, file: !3, line: 8, type: !12)
+!22 = distinct !DILexicalBlock(scope: !23, file: !3, line: 7, column: 10)
+!23 = distinct !DILexicalBlock(scope: !15, file: !3, line: 7, column: 7)
+!24 = !DILocation(line: 0, scope: !15)
+!25 = !DILocation(line: 6, column: 15, scope: !15)
+!26 = !{!27, !27, i64 0}
+!27 = !{!"int", !28, i64 0}
+!28 = !{!"omnipotent char", !29, i64 0}
+!29 = !{!"Simple C/C++ TBAA"}
+!30 = !DILocation(line: 6, column: 5, scope: !15)
+!31 = !{!32, !32, i64 0}
+!32 = !{!"short", !28, i64 0}
Index: llvm/lib/Transforms/Scalar/EarlyCSE.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -1136,8 +1136,12 @@
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
continue;
}
- if (!Inst.use_empty())
+ if (!Inst.use_empty()) {
+ // Salvage debug info early to be compatible with
+ // removeRedundantDbgInstrsUsingBackwardScan() used in SimplfyCFG.
+ salvageDebugInfoOrMarkUndef(Inst);
Inst.replaceAllUsesWith(Op);
+ }
salvageKnowledge(&Inst, &AC);
removeMSSA(Inst);
Inst.eraseFromParent();
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80691.267509.patch
Type: text/x-patch
Size: 4448 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200531/fa083378/attachment.bin>
More information about the llvm-commits
mailing list