[llvm] r310985 - Merge debug info when hoist then-else code to if.

[Robinson, Paul via llvm-commits]

Beyond that we get to the zero location - but in what scope?

The scope of where it's hoisted into; in this case, the scope of the branch instruction.  Which could be a parent of the common parent of the hoisted instruction, but so what.  The scope of a line-0 location is not especially relevant, and using the scope of where the instruction lands seems least disruptive.
--paulr

From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf Of David Blaikie via llvm-commits
Sent: Friday, August 25, 2017 5:10 PM
To: Adrian Prantl
Cc: llvm-commits at lists.llvm.org
Subject: Re: [llvm] r310985 - Merge debug info when hoist then-else code to if.


On Fri, Aug 25, 2017 at 4:44 PM Adrian Prantl <aprantl at apple.com<mailto:aprantl at apple.com>> wrote:
On Aug 25, 2017, at 3:15 PM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:

Adrian - what were we doing about cases where merged locations were on call instructions? I think this case is one of those, where if the instruction is a call, this may drop the location from the call, then inline through the call creating corrupted debug info.

I guess that in this case the only sane thing we can do is create a new artificial line 0 location in one of the two calls' scope. It's not pretty, but that;s the best I can come up with.

A few options:

Further up in the same file, the code avoids merging if the instruction is a call (originally r380995) SimplifyCFG.cpp:1282.

So that's one option - do nothing on calls. But it's probably not ideal/wrong (since it's not either location)

Beyond that we get to the zero location - but in what scope?
The root scope of the function? (what if these two calls are both within a single inlined call? That'll punch a hole in the range which isn't entirely accurate) The nearest common parent scope to the two instructions (assuming they both have debug locations? If one doesn't have a location, could we reliably use the other location? Probably not - so if one doesn't have a location we should use the zero location at the root of the current function for sure)
Worth finding the common parent scope?
Should this functionality be built into getMergedLocation?


-- adrian



Here's a test case that seems to exercise/assert on that (but it asserts relatively late - would be handy to have a way to run the verifier between each optimization - maybe there is such an option?):
struct string {
  ~string();
};
void f2();
void f1(int) { f2(); }
void run(int c) {
  string body;
  while (true) {
    if (c)
      f1(c);
    else
      f1(c);
  }
}

On Tue, Aug 15, 2017 at 6:56 PM Dehao Chen via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
Author: dehao
Date: Tue Aug 15 18:55:26 2017
New Revision: 310985

URL: http://llvm.org/viewvc/llvm-project?rev=310985&view=rev
Log:
Merge debug info when hoist then-else code to if.

Summary: When we move then-else code to if, we need to merge its debug info, otherwise the hoisted instruction may have inaccurate debug info attached.

Reviewers: aprantl, probinson, dblaikie, echristo, loladiro

Reviewed By: aprantl

Subscribers: sanjoy, llvm-commits

Differential Revision: https://reviews.llvm.org/D36778

Added:
    llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll
Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=310985&r1=310984&r2=310985&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Tue Aug 15 18:55:26 2017
@@ -1336,6 +1336,8 @@ HoistTerminator:
     I2->replaceAllUsesWith(NT);
     NT->takeName(I1);
   }
+  NT->setDebugLoc(DILocation::getMergedLocation(
+      I1->getDebugLoc(), I2->getDebugLoc()));

   IRBuilder<NoFolder> Builder(NT);
   // Hoisting one of the terminators from our successor is a great thing.

Added: llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll?rev=310985&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll (added)
+++ llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll Tue Aug 15 18:55:26 2017
@@ -0,0 +1,39 @@
+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+; Checks if the debug info is removed for the "select" instruction.
+; CHECK: cmp {{.*}} !dbg
+; CHECK-NOT: select {{.*}} !dbg
+define i32 @min(i32 %a, i32 %b) {
+entry:
+  %cmp = icmp slt i32 %a, %b, !dbg !9
+  br i1 %cmp, label %if.then, label %if.else, !dbg !10
+
+if.then:
+  br label %return, !dbg !11
+
+if.else:
+  br label %return, !dbg !12
+
+return:
+  %retval.0 = phi i32 [ %a, %if.then ], [ %b, %if.else ]
+  ret i32 %retval.0, !dbg !13
+}
+
+!llvm.dbg.cu<http://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 version 6.0.0 (trunk 310792)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "min.cc<http://min.cc>", 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 6.0.0 (trunk 310792)"}
+!7 = distinct !DISubprogram(name: "min", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 4, column: 8, scope: !7)
+!10 = !DILocation(line: 4, column: 6, scope: !7)
+!11 = !DILocation(line: 5, column: 3, scope: !7)
+!12 = !DILocation(line: 7, column: 3, scope: !7)
+!13 = !DILocation(line: 9, column: 1, scope: !7)


_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170826/9330c7e3/attachment.html>