<div dir="ltr"><div>My patch </div><a href="https://reviews.llvm.org/D44809">https://reviews.llvm.org/D44809</a><br><div><br></div><div>will fix the option comments.</div><div><br></div><div>I will commit it soon.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Mar 27, 2018 at 10:59 AM Davide Italiano <<a href="mailto:davide@freebsd.org">davide@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Mar 27, 2018 at 10:56 AM, Xinliang David Li<br>
<<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>> wrote:<br>
><br>
><br>
> On Tue, Mar 27, 2018 at 10:21 AM, Davide Italiano <<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>><br>
> wrote:<br>
>><br>
>> On Fri, Mar 23, 2018 at 10:06 AM, Xinliang David Li<br>
>> <<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > On Thu, Jul 20, 2017 at 1:43 PM, Davide Italiano via llvm-commits<br>
>> > <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> >><br>
>> >> Author: davide<br>
>> >> Date: Thu Jul 20 13:43:05 2017<br>
>> >> New Revision: 308668<br>
>> >><br>
>> >> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=308668&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=308668&view=rev</a><br>
>> >> Log:<br>
>> >> [PGO] Move the PGOInstrumentation pass to new OptRemark API.<br>
>> >><br>
>> >> This fixes PR33791.<br>
>> >><br>
>> >> Modified:<br>
>> >> llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp<br>
>> >> llvm/trunk/test/Transforms/PGOProfile/branch1.ll<br>
>> >><br>
>> >> Modified:<br>
>> >> llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp<br>
>> >> URL:<br>
>> >><br>
>> >> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp?rev=308668&r1=308667&r2=308668&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp?rev=308668&r1=308667&r2=308668&view=diff</a><br>
>> >><br>
>> >><br>
>> >> ==============================================================================<br>
>> >> --- llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp<br>
>> >> (original)<br>
>> >> +++ llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp<br>
>> >> Thu<br>
>> >> Jul 20 13:43:05 2017<br>
>> >> @@ -59,6 +59,7 @@<br>
>> >> #include "llvm/Analysis/CFG.h"<br>
>> >> #include "llvm/Analysis/IndirectCallSiteVisitor.h"<br>
>> >> #include "llvm/Analysis/LoopInfo.h"<br>
>> >> +#include "llvm/Analysis/OptimizationDiagnosticInfo.h"<br>
>> >> #include "llvm/IR/CallSite.h"<br>
>> >> #include "llvm/IR/DiagnosticInfo.h"<br>
>> >> #include "llvm/IR/Dominators.h"<br>
>> >> @@ -1483,10 +1484,9 @@ void setProfMetadata(Module *M, Instruct<br>
>> >> OS << " (total count : " << TotalCount << ")";<br>
>> >> OS.flush();<br>
>> >> Function *F = TI->getParent()->getParent();<br>
>> >> - emitOptimizationRemarkAnalysis(<br>
>> >> - F->getContext(), "pgo-use-annot", *F, TI->getDebugLoc(),<br>
>> >> - Twine(BrCondStr) +<br>
>> >> - " is true with probability : " + Twine(BranchProbStr));<br>
>> >> + OptimizationRemarkEmitter ORE(F);<br>
>> >> + ORE.emit(OptimizationRemark(DEBUG_TYPE, "pgo-instrumentation", TI)<br>
>> >> + << BrCondStr << " is true with probability : " <<<br>
>> >> BranchProbStr);<br>
>> >> }<br>
>> >> }<br>
>> >><br>
>> ><br>
>> ><br>
>> > Davide, I just noticed this change. This message is not about<br>
>> > transformations nor missed optimizations, so it should remain as<br>
>> > 'remark-analysis'. Also the pass name should not be<br>
>> > 'pgo-instrumentation'<br>
>> > because it is only meaningful during profile annotation pass. Any<br>
>> > reason<br>
>> > for the underlying change?<br>
>> ><br>
>><br>
>> (sorry for the late answer)<br>
>> This was supposed to be NFC, so, no, this wasn't intended.<br>
>><br>
>> I assume we<br>
>> didn't have enough coverage to find out this actually broke?<br>
>><br>
><br>
> The test case was also modified :). Also this option is a performance debug<br>
> option (pgo related), so yes, there is no better way to catch the break (not<br>
> really a break, but a change).<br>
><br>
<br>
I would like to apologize, I missed I also modified the test. Anyway,<br>
I can probably revert this or look over the weekend (FWIW, if you have<br>
urgency and want to look at this earlier, be my guest).<br>
<br>
Thanks,<br>
<br>
--<br>
Davide<br>
</blockquote></div>