<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 28, 2016 at 2:18 PM Dehao Chen <<a href="mailto:dehao@google.com">dehao@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg">I thought NFC applies to "obvious change to fix corner-case bugs", and apparently I was wrong and I'm sorry about that.</div></blockquote><div><br></div><div>Ah, right - NFC means No Functional Change. We use this to tag pure refactoring changes that do not change the observable behavior of the component in question. (eg: changing the API of SmallVector might not change the observable behavior of LLVM, but it probably still wouldn't be tagged NFC, because we tend to unit test ADTs directly - but changing some pass so that it still optimizes in the same way, but doesn't, say, build some intermediate data structure anymore, would be NFC because at the level we test that pass, nothing has changed)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"> What's the recommend steps to move forward for situations like this? Shall I revert the patch and send out a code review?</div></blockquote><div><br></div><div>Doesn't seem like there's anything needed to be done - just an FYI of don't use 'NFC' when semantics/behavior do change. You included what appears to be appropriate test coverage, etc, so no worries there.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Mon, Nov 28, 2016 at 10:04 AM, David Blaikie <span dir="ltr" class="gmail_msg"><<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br class="gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="gmail_msg"><div dir="ltr" class="gmail_msg">On Tue, Nov 22, 2016 at 2:59 PM Dehao Chen via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dehao<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Date: Tue Nov 22 16:50:01 2016<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
New Revision: 287710<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=287710&view=rev" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=287710&view=rev</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Log:<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Before sample pgo annotation, do not inline a function that has no debug info. (NFC)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg"></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></span><div class="gmail_msg">(+1 to Paul's comment - this sounds like a change, has test changes, and is probably not NFC)<br class="gmail_msg"><br class="gmail_msg">Also: This sounds like it does the opposite of what we want as a requirement: that the presence or absence of debug info should not change the resulting code.<br class="gmail_msg"><br class="gmail_msg">What's the deal?</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">This particular pass requires debug info to make decisions.</div></div></div></div></blockquote><div><br></div><div>Sorry, clearly not my area of understanding/expertise - could you give a brief description of why/how that is?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> This is mandated by pass manager: even with -g0, PM will make sure debug info is available before this pass.</div></div></div></div></blockquote><div><br></div><div>The PM doesn't produce debug info, though - the client building IR (Clang, etc) does.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> But if user manually removed the debug info, the code should also handle it correctly, which is fixed by this patch.</div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Dehao</div></div></div></div><div dir="ltr" class="gmail_msg"><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><div class="m_-5187531742590592069h5 gmail_msg"><div class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
If there is no debug info in the callee, inlining it will not help annotator. This avoids infinite loop as reported in PR/31119.<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Added:<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
    llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
    llvm/trunk/test/Transforms/SampleProfile/nodebug.ll<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Modified:<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
    llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
    llvm/trunk/test/Transforms/SampleProfile/early-inline.ll<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Modified: llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp?rev=287710&r1=287709&r2=287710&view=diff" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp?rev=287710&r1=287709&r2=287710&view=diff</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
==============================================================================<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
--- llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp (original)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+++ llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp Tue Nov 22 16:50:01 2016<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
@@ -651,6 +651,8 @@ bool SampleProfileLoader::inlineHotFunct<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
       InvokeInst *II = dyn_cast<InvokeInst>(I);<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
       Function *CalledFunction =<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
           (CI == nullptr ? II->getCalledFunction() : CI->getCalledFunction());<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+      if (!CalledFunction || !CalledFunction->getSubprogram())<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+        continue;<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
       DebugLoc DLoc = I->getDebugLoc();<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
       uint64_t NumSamples = findCalleeFunctionSamples(*I)->getTotalSamples();<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
       if ((CI && InlineFunction(CI, IFI)) || (II && InlineFunction(II, IFI))) {<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Added: llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof?rev=287710&view=auto" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof?rev=287710&view=auto</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
==============================================================================<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
--- llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof (added)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+++ llvm/trunk/test/Transforms/SampleProfile/Inputs/nodebug.prof Tue Nov 22 16:50:01 2016<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
@@ -0,0 +1,2 @@<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+foo:100:10<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+ 0: bar:10<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Modified: llvm/trunk/test/Transforms/SampleProfile/early-inline.ll<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/early-inline.ll?rev=287710&r1=287709&r2=287710&view=diff" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/early-inline.ll?rev=287710&r1=287709&r2=287710&view=diff</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
==============================================================================<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
--- llvm/trunk/test/Transforms/SampleProfile/early-inline.ll (original)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+++ llvm/trunk/test/Transforms/SampleProfile/early-inline.ll Tue Nov 22 16:50:01 2016<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
@@ -28,7 +28,7 @@ define void @_Z3foov() #0 personality i8<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 }<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 ; Function Attrs: nounwind uwtable<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
-define internal void @_ZL3barv() #1 {<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+define internal void @_ZL3barv() !dbg !12 {<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
   ret void<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 }<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
@@ -45,3 +45,4 @@ declare i32 @__gxx_personality_v0(...)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 !9 = !DILocation(line: 6, column: 3, scope: !6)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 !10 = !DILocation(line: 8, column: 5, scope: !11)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
 !11 = distinct !DILexicalBlock(scope: !6, file: !1, line: 7, column: 7)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!12 = distinct !DISubprogram(linkageName: "_ZL3barv", scope: !1, line: 20, scopeLine: 20, unit: !0)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
Added: llvm/trunk/test/Transforms/SampleProfile/nodebug.ll<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/nodebug.ll?rev=287710&view=auto" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SampleProfile/nodebug.ll?rev=287710&view=auto</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
==============================================================================<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
--- llvm/trunk/test/Transforms/SampleProfile/nodebug.ll (added)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+++ llvm/trunk/test/Transforms/SampleProfile/nodebug.ll Tue Nov 22 16:50:01 2016<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
@@ -0,0 +1,20 @@<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/nodebug.prof<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+define void @foo() !dbg !3 {<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+  call void @bar(), !dbg !4<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+  ret void<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+}<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+define void @bar() {<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+  call void @bar()<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+  ret void<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+}<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!<a href="http://llvm.dbg.cu" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">llvm.dbg.cu</a> = !{!0}<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!llvm.module.flags = !{!2}<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!1 = !DIFile(filename: "t", directory: "/tmp/")<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!2 = !{i32 2, !"Debug Info Version", i32 3}<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!3 = distinct !DISubprogram(name: "a", scope: !1, file: !1, line: 10, unit: !0)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
+!4 = !DILocation(line: 10, scope: !3)<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
_______________________________________________<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
llvm-commits mailing list<br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="m_-5187531742590592069m_1066472164475782851gmail_msg gmail_msg">
</blockquote></div></div></div></div>
</blockquote></div></div></div></blockquote></div></div>