[PATCH] D58403: [DebugInfo][CGP] Update dbg.values when updating memory address computations

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: aprantl, bjope, vsk.
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

One of CodeGenPrepare's optimisations is to duplicate address calculations into basic blocks, so that as much information as possible can be folded into memory addressing operands. This is great -- but the dbg.value variable location intrinsics are not updated in the same way. This can lead to dbg.values referring to address computations in other blocks that will never be encoded into the DAG, while duplicate address computations are performed locally that could be used by the dbg.value. Some of these (such as non-constant-offset GEPs)  can't be salvaged past.

Fix this by, whenever we duplicate an address computation into a block, looking for dbg.value users of the original memory address in the same block, and redirecting those to the local computation.


Repository:
  rL LLVM

https://reviews.llvm.org/D58403

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/DebugInfo/Generic/codegenprep-addrsink.ll


Index: test/DebugInfo/Generic/codegenprep-addrsink.ll
===================================================================
--- /dev/null
+++ test/DebugInfo/Generic/codegenprep-addrsink.ll
@@ -0,0 +1,53 @@
+; RUN: llc -start-before=codegenprepare -stop-after=codegenprepare %s -o - | FileCheck %s
+;
+; CGP duplicates address calculation into each basic block that contains loads
+; or stores, so that they can be folded into instruction memory operands for
+; example. dbg.value's should be redirected to identify such local address
+; computations, to give the best opportunity for variable locations to be
+; preserved.
+
+define dso_local i8 @foo(i32 *%p, i32 %cond) !dbg !7 {
+entry:
+  %casted = bitcast i32 *%p to i8*
+  %arith = getelementptr i8, i8 *%casted, i32 3
+  %cmpresult = icmp eq i32 %cond, 0
+  br i1 %cmpresult, label %next, label %ret
+
+next:
+; Address calcs should be duplicated into this block, and the dbg.value
+; should refer to the address calculated _in_ the block.
+; CHECK-LABEL: next:
+; CHECK:       %[[CASTVAR:[0-9a-zA-Z]+]] = bitcast i32* %p to i8*
+; CHECK-NEXT:  %[[GEPVAR:[0-9a-zA-Z]+]] = getelementptr i8, i8* %[[CASTVAR]], i64 3
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata i8* %[[GEPVAR]], metadata ![[DIVAR:[0-9]+]],
+  call void @llvm.dbg.value(metadata i8 *%arith, metadata !12, metadata !DIExpression()), !dbg !14
+  %loaded = load i8, i8 *%arith
+  ret i8 %loaded
+
+ret:
+  ret i8 0
+}
+
+; CHECK: ![[DIVAR]] = !DILocalVariable(name: "p",
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 8.0.0 (trunk 348209)"}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 4, type: !8, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{null, !10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!12}
+!12 = !DILocalVariable(name: "p", arg: 1, scope: !7, file: !1, line: 4, type: !10)
+!14 = !DILocation(line: 4, column: 15, scope: !7)
+!20 = distinct !DILexicalBlock(scope: !7, file: !1, line: 8, column: 7)
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -4929,6 +4929,19 @@
   // value, this should be idempotent.
   SunkAddrs[Addr] = WeakTrackingVH(SunkAddr);
 
+  // Redirect debug intrinsic users in the local block to the locally computed
+  // address.
+  SmallVector<DbgVariableIntrinsic *, 4> DbgUsers;
+  findDbgUsers(DbgUsers, Repl);
+  for (auto &User : DbgUsers) {
+    // Discard debug users in other blocks.
+    if (User->getParent() != MemoryInst->getParent())
+      continue;
+
+    User->setOperand(0, MetadataAsValue::get(User->getContext(),
+                                             ValueAsMetadata::get(SunkAddr)));
+  };
+
   // If we have no uses, recursively delete the value and all dead instructions
   // using it.
   if (Repl->use_empty()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58403.187432.patch
Type: text/x-patch
Size: 3515 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190219/780be111/attachment.bin>