<div dir="ltr">FWIW, generally I'm a bit uncertain about changing/dropping locations to improve stepping behavior. If the optimizer did reorder instructions I think it's potentialyl valuable to show that.<br><br>The main (compelling to me, at least) motivation for stripping/shifting locations seems to be accurate profiles, which only applies when an instruction is moved between basic blocks - it sounds like that is the case here, so it's fine, but just thought I'd check. Is that the case that this fix/change applies to instructions moving between basic blocks?</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 8, 2017 at 4:17 PM Paul Robinson 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: probinson<br>
Date: Fri Dec 8 16:17:01 2017<br>
New Revision: 320216<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=320216&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=320216&view=rev</a><br>
Log:<br>
Fix out-of-order stepping behavior in programs with sunk instructions.<br>
<br>
MachineSink attempts to place instructions near the basic blocks where<br>
they are needed. Once an instruction has been sunk, its location<br>
relative to other instructions no longer is consistent with the<br>
original source code. In order to ensure correct stepping in the<br>
debugger, the debug location for sunk instructions is either merged<br>
with the insertion point or erased if the target successor block is<br>
empty.<br>
<br>
Originally submitted as r318679, revised to fix sanitizer failure and<br>
improve testing.<br>
<br>
Patch by Matthew Voss!<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D39933" rel="noreferrer" target="_blank">https://reviews.llvm.org/D39933</a><br>
<br>
Added:<br>
llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll<br>
llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll<br>
Modified:<br>
llvm/trunk/lib/CodeGen/MachineSink.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/MachineSink.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=320216&r1=320215&r2=320216&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineSink.cpp?rev=320216&r1=320215&r2=320216&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/MachineSink.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/MachineSink.cpp Fri Dec 8 16:17:01 2017<br>
@@ -38,6 +38,7 @@<br>
#include "llvm/CodeGen/TargetSubtargetInfo.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,11 +869,20 @@ bool MachineSinking::SinkInstruction(Mac<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() && InsertPos != SuccToSinkTo->end())<br>
+ MI.setDebugLoc(DILocation::getMergedLocation(MI.getDebugLoc(),<br>
+ InsertPos->getDebugLoc()));<br>
+ else<br>
+ MI.setDebugLoc(DebugLoc());<br>
+<br>
+<br>
// Move the instruction.<br>
SuccToSinkTo->splice(InsertPos, ParentBlock, MI,<br>
++MachineBasicBlock::iterator(MI));<br>
<br>
- // Move debug values.<br>
+ // Move previously adjacent debug value instructions to the insert position.<br>
for (SmallVectorImpl<MachineInstr *>::iterator DBI = DbgValuesToSink.begin(),<br>
DBE = DbgValuesToSink.end(); DBI != DBE; ++DBI) {<br>
MachineInstr *DbgMI = *DBI;<br>
<br>
Added: llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll?rev=320216&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll?rev=320216&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll (added)<br>
+++ llvm/trunk/test/CodeGen/X86/machinesink-merge-debuginfo.ll Fri Dec 8 16:17:01 2017<br>
@@ -0,0 +1,56 @@<br>
+; RUN: llc -simplify-mir -stop-after=machine-sink < %s -o - | 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 !16<br>
+ tail call void @llvm.dbg.value(metadata i1 %c, metadata !15, metadata !DIExpression()), !dbg !16<br>
+ %a = fdiv double %x, 3.000000e+00<br>
+ %b = fdiv double %y, 5.000000e+00, !dbg !17<br>
+ br i1 %c, label %first, label %second<br>
+first:<br>
+ %e = fadd double %a, 1.000000e+00<br>
+ br label %final<br>
+second:<br>
+ %f = fadd double %b, 1.000000e+00, !dbg !17<br>
+; CHECK: debug-location !17<br>
+; CHECK: debug-location !17<br>
+ br label %final<br>
+final:<br>
+ %cond = phi double [%e, %first], [%f, %second]<br>
+ %d = fadd double %cond, 1.000000e+00<br>
+ ret double %d<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" 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_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: 2, column: 26, scope: !7)<br>
<br>
Added: llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll?rev=320216&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll?rev=320216&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll (added)<br>
+++ llvm/trunk/test/CodeGen/X86/machinesink-null-debuginfo.ll Fri Dec 8 16:17:01 2017<br>
@@ -0,0 +1,49 @@<br>
+; RUN: llc -simplify-mir -stop-after=machine-sink < %s -o - | 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: debug-location !21<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" 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_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>
<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>