<div dir="ltr">Out of curiosity - how does this handle cases of fragmented lexical scopes? (& what if, say, the DBG_VALUE starts outside any of the fragments of the lexical scope, but is live within it?)<br><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 3, 2017 at 4:54 AM Robert Lougher via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rlougher<br>
Date: Thu Aug  3 04:54:02 2017<br>
New Revision: 309933<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=309933&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=309933&view=rev</a><br>
Log:<br>
[LiveDebugVariables] Use lexical scope to trim debug value live intervals<br>
<br>
The debug value live intervals computed by Live Debug Variables may extend<br>
beyond the range of the debug location's lexical scope. In this case,<br>
splitting of an interval can result in an interval outside of the scope being<br>
created, causing extra unnecessary DBG_VALUEs to be emitted. To prevent this,<br>
trim the intervals to the lexical scope.<br>
<br>
This resolves PR33730.<br>
<br>
Reviewers: aprantl<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D35953" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35953</a><br>
<br>
Added:<br>
    llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll<br>
Modified:<br>
    llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp<br>
    llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=309933&r1=309932&r2=309933&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp?rev=309933&r1=309932&r2=309933&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/LiveDebugVariables.cpp Thu Aug  3 04:54:02 2017<br>
@@ -21,7 +21,9 @@<br>
<br>
 #include "LiveDebugVariables.h"<br>
 #include "llvm/ADT/IntervalMap.h"<br>
+#include "llvm/ADT/SmallSet.h"<br>
 #include "llvm/ADT/Statistic.h"<br>
+#include "llvm/CodeGen/LexicalScopes.h"<br>
 #include "llvm/CodeGen/LiveIntervalAnalysis.h"<br>
 #include "llvm/CodeGen/MachineDominators.h"<br>
 #include "llvm/CodeGen/MachineFunction.h"<br>
@@ -101,6 +103,10 @@ class UserValue {<br>
   /// Map of slot indices where this value is live.<br>
   LocMap locInts;<br>
<br>
+  /// Set of interval start indexes that have been trimmed to the<br>
+  /// lexical scope.<br>
+  SmallSet<SlotIndex, 2> trimmedDefs;<br>
+<br>
   /// coalesceLocation - After LocNo was changed, check if it has become<br>
   /// identical to another location, and coalesce them. This may cause LocNo or<br>
   /// a later location to be erased, but no earlier location will be erased.<br>
@@ -229,7 +235,7 @@ public:<br>
   /// computeIntervals - Compute the live intervals of all locations after<br>
   /// collecting all their def points.<br>
   void computeIntervals(MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,<br>
-                        LiveIntervals &LIS);<br>
+                        LiveIntervals &LIS, LexicalScopes &LS);<br>
<br>
   /// splitRegister - Replace OldReg ranges with NewRegs ranges where NewRegs is<br>
   /// live. Returns true if any changes were made.<br>
@@ -627,10 +633,9 @@ UserValue::addDefsFromCopies(LiveInterva<br>
   }<br>
 }<br>
<br>
-void<br>
-UserValue::computeIntervals(MachineRegisterInfo &MRI,<br>
-                            const TargetRegisterInfo &TRI,<br>
-                            LiveIntervals &LIS) {<br>
+void UserValue::computeIntervals(MachineRegisterInfo &MRI,<br>
+                                 const TargetRegisterInfo &TRI,<br>
+                                 LiveIntervals &LIS, LexicalScopes &LS) {<br>
   SmallVector<std::pair<SlotIndex, unsigned>, 16> Defs;<br>
<br>
   // Collect all defs to be extended (Skipping undefs).<br>
@@ -672,17 +677,88 @@ UserValue::computeIntervals(MachineRegis<br>
     extendDef(Idx, LocNo, LR, VNI, nullptr, LIS);<br>
   }<br>
<br>
-  // Finally, erase all the undefs.<br>
+  // Erase all the undefs.<br>
   for (LocMap::iterator I = locInts.begin(); I.valid();)<br>
     if (I.value() == ~0u)<br>
       I.erase();<br>
     else<br>
       ++I;<br>
+<br>
+  // The computed intervals may extend beyond the range of the debug<br>
+  // location's lexical scope. In this case, splitting of an interval<br>
+  // can result in an interval outside of the scope being created,<br>
+  // causing extra unnecessary DBG_VALUEs to be emitted. To prevent<br>
+  // this, trim the intervals to the lexical scope.<br>
+<br>
+  LexicalScope *Scope = LS.findLexicalScope(dl);<br>
+  if (!Scope)<br>
+    return;<br>
+<br>
+  SlotIndex PrevEnd;<br>
+  LocMap::iterator I = locInts.begin();<br>
+<br>
+  // Iterate over the lexical scope ranges. Each time round the loop<br>
+  // we check the intervals for overlap with the end of the previous<br>
+  // range and the start of the next. The first range is handled as<br>
+  // a special case where there is no PrevEnd.<br>
+  for (const InsnRange &Range : Scope->getRanges()) {<br>
+    SlotIndex RStart = LIS.getInstructionIndex(*Range.first);<br>
+    SlotIndex REnd = LIS.getInstructionIndex(*Range.second);<br>
+<br>
+    // At the start of each iteration I has been advanced so that<br>
+    // I.stop() >= PrevEnd. Check for overlap.<br>
+    if (PrevEnd && I.start() < PrevEnd) {<br>
+      SlotIndex IStop = I.stop();<br>
+      unsigned LocNo = I.value();<br>
+<br>
+      // Stop overlaps previous end - trim the end of the interval to<br>
+      // the scope range.<br>
+      I.setStopUnchecked(PrevEnd);<br>
+      ++I;<br>
+<br>
+      // If the interval also overlaps the start of the "next" (i.e.<br>
+      // current) range create a new interval for the remainder (which<br>
+      // may be further trimmed).<br>
+      if (RStart < IStop)<br>
+        I.insert(RStart, IStop, LocNo);<br>
+    }<br>
+<br>
+    // Advance I so that I.stop() >= RStart, and check for overlap.<br>
+    I.advanceTo(RStart);<br>
+    if (!I.valid())<br>
+      return;<br>
+<br>
+    if (I.start() < RStart) {<br>
+      // Interval start overlaps range - trim to the scope range.<br>
+      I.setStartUnchecked(RStart);<br>
+      // Remember that this interval was trimmed.<br>
+      trimmedDefs.insert(RStart);<br>
+    }<br>
+<br>
+    // The end of a lexical scope range is the last instruction in the<br>
+    // range. To convert to an interval we need the index of the<br>
+    // instruction after it.<br>
+    REnd = REnd.getNextIndex();<br>
+<br>
+    // Advance I to first interval outside current range.<br>
+    I.advanceTo(REnd);<br>
+    if (!I.valid())<br>
+      return;<br>
+<br>
+    PrevEnd = REnd;<br>
+  }<br>
+<br>
+  // Check for overlap with end of final range.<br>
+  if (PrevEnd && I.start() < PrevEnd)<br>
+    I.setStopUnchecked(PrevEnd);<br>
 }<br>
<br>
 void LDVImpl::computeIntervals() {<br>
+  LexicalScopes LS;<br>
+  LS.initialize(*MF);<br>
+<br>
   for (unsigned i = 0, e = userValues.size(); i != e; ++i) {<br>
-    userValues[i]->computeIntervals(MF->getRegInfo(), *TRI, *LIS);<br>
+    userValues[i]->computeIntervals(MF->getRegInfo(), *TRI, *LIS, LS);<br>
     userValues[i]->mapVirtRegs(this);<br>
   }<br>
 }<br>
@@ -957,6 +1033,13 @@ void UserValue::emitDebugValues(VirtRegM<br>
     SlotIndex Start = I.start();<br>
     SlotIndex Stop = I.stop();<br>
     unsigned LocNo = I.value();<br>
+<br>
+    // If the interval start was trimmed to the lexical scope insert the<br>
+    // DBG_VALUE at the previous index (otherwise it appears after the<br>
+    // first instruction in the range).<br>
+    if (trimmedDefs.count(Start))<br>
+      Start = Start.getPrevIndex();<br>
+<br>
     DEBUG(dbgs() << "\t[" << Start << ';' << Stop << "):" << LocNo);<br>
     MachineFunction::iterator MBB = LIS.getMBBFromIndex(Start)->getIterator();<br>
     SlotIndex MBBEnd = LIS.getMBBEndIdx(&*MBB);<br>
<br>
Modified: llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll?rev=309933&r1=309932&r2=309933&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll?rev=309933&r1=309932&r2=309933&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll (original)<br>
+++ llvm/trunk/test/DebugInfo/X86/dbg-value-dag-combine.ll Thu Aug  3 04:54:02 2017<br>
@@ -5,10 +5,11 @@ target triple = "i686-apple-darwin"<br>
<br>
 ; There should be a DEBUG_VALUE for each call to llvm.dbg.value<br>
<br>
-; CHECK:  ##DEBUG_VALUE: __OpenCL_test_kernel:ip <-<br>
-; CHECK:  ##DEBUG_VALUE: xxx <- 0<br>
-; CHECK:  ##DEBUG_VALUE: gid <- %E{{..$}}<br>
-; CHECK:  ##DEBUG_VALUE: idx <- %E{{..$}}<br>
+; CHECK-LABEL: __OpenCL_test_kernel:<br>
+; CHECK-DAG:  ##DEBUG_VALUE: __OpenCL_test_kernel:ip <-<br>
+; CHECK-DAG:  ##DEBUG_VALUE: xxx <- 0<br>
+; CHECK-DAG:  ##DEBUG_VALUE: gid <- %E{{..$}}<br>
+; CHECK-DAG:  ##DEBUG_VALUE: idx <- %E{{..$}}<br>
 ; CHECK-NOT:  ##DEBUG_VALUE:<br>
<br>
 declare <4 x i32> @__amdil_get_global_id_int()<br>
<br>
Added: llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll?rev=309933&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll?rev=309933&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll (added)<br>
+++ llvm/trunk/test/DebugInfo/X86/live-debug-variables.ll Thu Aug  3 04:54:02 2017<br>
@@ -0,0 +1,77 @@<br>
+; RUN: llc -mtriple=x86_64-linux-gnu -filetype=obj -o - %s | llvm-dwarfdump -debug-dump=loc - | FileCheck %s<br>
+<br>
+; The test inlines the function F four times, with each inlined variable for<br>
+; "i4" sharing the same virtual register. This means the live interval of the<br>
+; register spans all of the inlined callsites, extending beyond the lexical<br>
+; scope of each. Later during register allocation the live interval is split<br>
+; into multiple intervals. Check that this does not generate multiple entries<br>
+; within the debug location (see PR33730).<br>
+;<br>
+; Generated from:<br>
+;<br>
+; extern int foobar(int, int, int, int, int);<br>
+;<br>
+; int F(int i1, int i2, int i3, int i4, int i5) {<br>
+;   return foobar(i1, i2, i3, i4, i5);<br>
+; }<br>
+;<br>
+; int foo(int a, int b, int c, int d, int e) {<br>
+;   return F(a,b,c,d,e) +<br>
+;          F(a,b,c,d,e) +<br>
+;          F(a,b,c,d,e) +<br>
+;          F(a,b,c,d,e);<br>
+; }<br>
+<br>
+; CHECK: Beginning address offset<br>
+; CHECK-NOT: Beginning address offset<br>
+<br>
+declare i32 @foobar(i32, i32, i32, i32, i32)<br>
+<br>
+define i32 @foo(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) !dbg !25 {<br>
+entry:<br>
+  tail call void @llvm.dbg.value(metadata i32 %d, i64 0, metadata !15, metadata !17) #3, !dbg !41<br>
+  %call.i = tail call i32 @foobar(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) #3, !dbg !43<br>
+  %call.i21 = tail call i32 @foobar(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) #3, !dbg !50<br>
+  %add = add nsw i32 %call.i21, %call.i, !dbg !51<br>
+  %call.i22 = tail call i32 @foobar(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) #3, !dbg !58<br>
+  %add3 = add nsw i32 %add, %call.i22, !dbg !59<br>
+  %call.i23 = tail call i32 @foobar(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) #3, !dbg !66<br>
+  %add5 = add nsw i32 %add3, %call.i23, !dbg !67<br>
+  ret i32 %add5, !dbg !68<br>
+}<br>
+<br>
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata)<br>
+<br>
+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!3, !4, !5}<br>
+!llvm.ident = !{!6}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0 (trunk 308976)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)<br>
+!1 = !DIFile(filename: "foo.c", directory: "")<br>
+!2 = !{}<br>
+!3 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!5 = !{i32 1, !"wchar_size", i32 4}<br>
+!6 = !{!"clang version 6.0.0 (trunk 308976)"}<br>
+!7 = distinct !DISubprogram(name: "F", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !11)<br>
+!8 = !DISubroutineType(types: !9)<br>
+!9 = !{!10, !10, !10, !10, !10, !10}<br>
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)<br>
+!11 = !{!15}<br>
+!15 = !DILocalVariable(name: "i4", arg: 4, scope: !7, file: !1, line: 3, type: !10)<br>
+!17 = !DIExpression()<br>
+!25 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 7, type: !8, isLocal: false, isDefinition: true, scopeLine: 7, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !26)<br>
+!26 = !{}<br>
+!38 = distinct !DILocation(line: 8, column: 10, scope: !25)<br>
+!41 = !DILocation(line: 3, column: 35, scope: !7, inlinedAt: !38)<br>
+!43 = !DILocation(line: 4, column: 10, scope: !7, inlinedAt: !38)<br>
+!45 = distinct !DILocation(line: 9, column: 10, scope: !25)<br>
+!50 = !DILocation(line: 4, column: 10, scope: !7, inlinedAt: !45)<br>
+!51 = !DILocation(line: 8, column: 23, scope: !25)<br>
+!53 = distinct !DILocation(line: 10, column: 10, scope: !25)<br>
+!58 = !DILocation(line: 4, column: 10, scope: !7, inlinedAt: !53)<br>
+!59 = !DILocation(line: 9, column: 23, scope: !25)<br>
+!61 = distinct !DILocation(line: 11, column: 10, scope: !25)<br>
+!66 = !DILocation(line: 4, column: 10, scope: !7, inlinedAt: !61)<br>
+!67 = !DILocation(line: 10, column: 23, scope: !25)<br>
+!68 = !DILocation(line: 8, column: 3, scope: !25)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>