<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 12 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">That makes sense. I’ll add the test and resubmit.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Blaikie [mailto:dblaikie@gmail.com]
<br>
<b>Sent:</b> Monday, November 20, 2017 1:16 PM<br>
<b>To:</b> reviews+D39933+public+5dbf07e50e16ff55@reviews.llvm.org<br>
<b>Cc:</b> Voss, Matthew; aprantl@apple.com; Robinson, Paul; echristo@gmail.com; llvm-commits@lists.llvm.org<br>
<b>Subject:</b> Re: [PATCH] D39933: Summary: Fix out-of-order stepping behavior in programs with sunk instructions<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Should this code have more test coverage? Looks like it's only looking for the absence of a location. Should there be testing for the merge case as well?<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, Nov 10, 2017 at 5:11 PM Matthew Voss via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal" style="margin-bottom:12.0pt">ormris created this revision.<br>
<br>
MachineSink attempts to place instructions near the basic blocks where<br>
they are needed.<br>
<br>
Once an instruction has been sunk, its location relative to other<br>
instructions no longer is consistent with the original source code. In<br>
order to ensure correct stepping in the debugger, the debug location for<br>
sunk instructions is either merged with the insertion point or erased if the<br>
target successor block is empty.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D39933" target="_blank">https://reviews.llvm.org/D39933</a><br>
<br>
Files:<br>
lib/CodeGen/MachineSink.cpp<br>
test/CodeGen/X86/machinesink-debuginfo.ll<br>
<br>
<br>
Index: test/CodeGen/X86/machinesink-debuginfo.ll<br>
===================================================================<br>
--- /dev/null<br>
+++ test/CodeGen/X86/machinesink-debuginfo.ll<br>
@@ -0,0 +1,49 @@<br>
+; RUN: llc -O2 -o - < %s | FileCheck %s<br>
+<br>
+; ModuleID = 'test-sink-debug.cpp'<br>
+source_filename = "test-sink-debug.cpp"<br>
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+; Function Attrs: nounwind readnone uwtable<br>
+define double @_Z3fooddb(double %x, double %y, i1 zeroext %c) local_unnamed_addr !dbg !7 {<br>
+ tail call void @llvm.dbg.value(metadata double %x, metadata !13, metadata !DIExpression()), !dbg !16<br>
+ tail call void @llvm.dbg.value(metadata double %y, metadata !14, metadata !DIExpression()), !dbg !17<br>
+ tail call void @llvm.dbg.value(metadata i1 %c, metadata !15, metadata !DIExpression()), !dbg !18<br>
+ %a = fdiv double %x, 3.000000e+00<br>
+ %b = fdiv double %y, 5.000000e+00, !dbg !21<br>
+ %cond = select i1 %c, double %a, double %b<br>
+; CHECK-NOT: .loc 1 2 26<br>
+ ret double %cond, !dbg !22<br>
+}<br>
+<br>
+; Function Attrs: nounwind readnone speculatable<br>
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1<br>
+<br>
+attributes #1 = { nounwind readnone speculatable }<br>
+<br>
+!<a href="http://llvm.dbg.cu" 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_C_plus_plus, file: !1, producer: "clang version 6.0.0 (trunk 313291)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)<br>
+!1 = !DIFile(filename: "test-sink-debug.cpp", directory: "/tmp")<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 313291)"}<br>
+!7 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooddb", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)<br>
+!8 = !DISubroutineType(types: !9)<br>
+!9 = !{!10, !10, !10, !11}<br>
+!10 = !DIBasicType(name: "double", size: 64, encoding: DW_ATE_float)<br>
+!11 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)<br>
+!12 = !{!13, !14, !15}<br>
+!13 = !DILocalVariable(name: "x", arg: 1, scope: !7, file: !1, line: 1, type: !10)<br>
+!14 = !DILocalVariable(name: "y", arg: 2, scope: !7, file: !1, line: 1, type: !10)<br>
+!15 = !DILocalVariable(name: "c", arg: 3, scope: !7, file: !1, line: 1, type: !11)<br>
+!16 = !DILocation(line: 1, column: 19, scope: !7)<br>
+!17 = !DILocation(line: 1, column: 29, scope: !7)<br>
+!18 = !DILocation(line: 1, column: 37, scope: !7)<br>
+!21 = !DILocation(line: 2, column: 26, scope: !7)<br>
+!22 = !DILocation(line: 2, column: 3, scope: !7)<br>
Index: lib/CodeGen/MachineSink.cpp<br>
===================================================================<br>
--- lib/CodeGen/MachineSink.cpp<br>
+++ lib/CodeGen/MachineSink.cpp<br>
@@ -36,6 +36,7 @@<br>
#include "llvm/CodeGen/TargetInstrInfo.h"<br>
#include "llvm/IR/BasicBlock.h"<br>
#include "llvm/IR/LLVMContext.h"<br>
+#include "llvm/IR/DebugInfoMetadata.h"<br>
#include "llvm/Pass.h"<br>
#include "llvm/Support/BranchProbability.h"<br>
#include "llvm/Support/CommandLine.h"<br>
@@ -868,6 +869,16 @@<br>
SmallVector<MachineInstr *, 2> DbgValuesToSink;<br>
collectDebugValues(MI, DbgValuesToSink);<br>
<br>
+ // Merge or erase debug location to ensure consistent stepping in profilers<br>
+ // and debuggers.<br>
+ if (!SuccToSinkTo->empty()) {<br>
+ MI.setDebugLoc(DILocation::getMergedLocation(MI.getDebugLoc(),<br>
+ InsertPos->getDebugLoc()));<br>
+ } else {<br>
+ MI.setDebugLoc(DebugLoc());<br>
+ }<br>
+<br>
+<br>
// Move the instruction.<br>
SuccToSinkTo->splice(InsertPos, ParentBlock, MI,<br>
++MachineBasicBlock::iterator(MI));<br>
<br>
<o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</body>
</html>