<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;}
/* 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">
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Does this only apply to cases that would be a problem for profile accuracy - or does it apply in cases where code is moved within a single basic block? (not sure which kind of moves GVN/PRE do nor which ones your change is proposing to
change the debug info for)<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">These are only cases where instructions are moved to or materialized in different basic blocks, so they could potentially affect profile accuracy.<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>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-- wolfgang<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"> <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"><br>
The modified test case phi-translate.ll covers all cases.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D27857" target="_blank">https://reviews.llvm.org/D27857</a><br>
<br>
Files:<br>
lib/Transforms/Scalar/GVN.cpp<br>
test/Transforms/GVN/PRE/phi-translate.ll<br>
<br>
<br>
Index: test/Transforms/GVN/PRE/phi-translate.ll<br>
===================================================================<br>
--- test/Transforms/GVN/PRE/phi-translate.ll<br>
+++ test/Transforms/GVN/PRE/phi-translate.ll<br>
@@ -4,18 +4,17 @@<br>
<br>
; CHECK-LABEL: @foo(<br>
; CHECK: entry.end_crit_edge:<br>
-; CHECK: %j.phi.trans.insert = sext i32 %x to i64, !dbg [[J_LOC:![0-9]+]]<br>
-; CHECK: %q.phi.trans.insert = getelementptr {{.*}}, !dbg [[Q_LOC:![0-9]+]]<br>
-; CHECK: %n.pre = load i32, i32* %q.phi.trans.insert, !dbg [[N_LOC:![0-9]+]]<br>
+; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{$}}<br>
+; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{$}}<br>
+; CHECK: %n.pre = load i32, i32* %[[ADDRESS]]{{$}}<br>
+; CHECK: br label %end<br>
; CHECK: then:<br>
; CHECK: store i32 %z<br>
; CHECK: end:<br>
-; CHECK: %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC]]<br>
+; CHECK: %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC:![0-9]+]]<br>
; CHECK: ret i32 %n<br>
<br>
-; CHECK-DAG: [[J_LOC]] = !DILocation(line: 45, column: 1, scope: !{{.*}})<br>
-; CHECK-DAG: [[Q_LOC]] = !DILocation(line: 46, column: 1, scope: !{{.*}})<br>
-; CHECK-DAG: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})<br>
+; CHECK: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}})<br>
<br>
@G = external global [100 x i32]<br>
define i32 @foo(i32 %x, i32 %z) !dbg !6 {<br>
Index: lib/Transforms/Scalar/GVN.cpp<br>
===================================================================<br>
--- lib/Transforms/Scalar/GVN.cpp<br>
+++ lib/Transforms/Scalar/GVN.cpp<br>
@@ -1572,6 +1572,13 @@<br>
<br>
// Assign value numbers to the new instructions.<br>
for (Instruction *I : NewInsts) {<br>
+ // Instructions that have been inserted in predecessor(s) to materialize<br>
+ // the load address do not retain their original debug locations. Doing<br>
+ // so could lead to erratic source attributions.<br>
+ // FIXME: How do we retain source locations without causing poor debugging<br>
+ // behavior?<br>
+ I->setDebugLoc(DebugLoc());<br>
+<br>
// FIXME: We really _ought_ to insert these value numbers into their<br>
// parent's availability map. However, in doing so, we risk getting into<br>
// ordering issues. If a block hasn't been processed yet, we would be<br>
@@ -1601,8 +1608,10 @@<br>
if (auto *RangeMD = LI->getMetadata(LLVMContext::MD_range))<br>
NewLoad->setMetadata(LLVMContext::MD_range, RangeMD);<br>
<br>
- // Transfer DebugLoc.<br>
- NewLoad->setDebugLoc(LI->getDebugLoc());<br>
+ // We do not propagate the old load's debug location, because the new<br>
+ // load now lives in a different BB.<br>
+ // FIXME: How do we retain source locations without causing poor debugging<br>
+ // behavior?<br>
<br>
// Add the newly created load.<br>
ValuesPerBlock.push_back(AvailableValueInBlock::get(UnavailablePred,<br>
<br>
<o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</div>
</body>
</html>