<div dir="ltr">It was deliberate to make it be zero cost in release builds, because it just compiles out.<div>This meant you could use it anywhere without fear it would slow things down.<br><div><br></div><div>But, if we don't like the tradeoff, i'm not opposed to other ones.</div></div><div><br></div><div>For example, enabling in all assertion builds, etc.</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 24, 2017 at 7:16 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">That's fine, though a bit unfortunate. It looks like it just returns true for shouldExecute in non-asserts builds. Danny, was there a particular reason you did this? As long as the default skip=0 and count=-1 it should be pretty harmless to have them on?<span class="HOEnZb"><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 24, 2017 at 5:21 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It turns out debug-counter currently is only enabled for debug build -- there might be some compile time concern (depending on the granularity of control). For now I will keep the current implementation unless someone feels strongly about it.<span class="m_4244195221455219263HOEnZb"><font color="#888888"><div><br></div><div>David</div></font></span></div><div class="m_4244195221455219263HOEnZb"><div class="m_4244195221455219263h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Apr 23, 2017 at 8:00 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I agree, this should use debug counters.<br>
<div class="m_4244195221455219263m_6142482392848763472HOEnZb"><div class="m_4244195221455219263m_6142482392848763472h5"><br>
On Sun, Apr 23, 2017 at 7:29 PM, Sean Silva via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
> Maybe use the DebugCounter mechanism Danny added? (see e.g. r295594 and<br>
> <a href="http://llvm.org/docs/ProgrammersManual.html#adding-debug-counters-to-aid-in-debugging-your-code" rel="noreferrer" target="_blank">http://llvm.org/docs/Programme<wbr>rsManual.html#adding-debug-cou<wbr>nters-to-aid-in-debugging-your<wbr>-code</a><br>
> ).<br>
><br>
> -- Sean Silva<br>
><br>
> On Sun, Apr 23, 2017 at 4:39 PM, Xinliang David Li via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: davidxl<br>
>> Date: Sun Apr 23 18:39:04 2017<br>
>> New Revision: 301148<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=301148&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=301148&view=rev</a><br>
>> Log:<br>
>> [PartialInine]: add triaging options<br>
>><br>
>> There are more bugs (runtime failures) triggered when partial<br>
>> inlining is turned on. Add options to help triaging problems.<br>
>><br>
>> Added:<br>
>> llvm/trunk/test/Transforms/Co<wbr>deExtractor/PartialInlineOptRe<wbr>mark.ll<br>
>> Modified:<br>
>> llvm/trunk/lib/Transforms/IPO<wbr>/PartialInlining.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/IPO/<wbr>PartialInlining.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp?rev=301148&r1=301147&r2=301148&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/IPO/PartialInlining.cpp?rev=<wbr>301148&r1=301147&r2=301148&vie<wbr>w=diff</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/lib/Transforms/IPO/<wbr>PartialInlining.cpp (original)<br>
>> +++ llvm/trunk/lib/Transforms/IPO/<wbr>PartialInlining.cpp Sun Apr 23 18:39:04<br>
>> 2017<br>
>> @@ -33,6 +33,17 @@ using namespace llvm;<br>
>><br>
>> STATISTIC(NumPartialInlined, "Number of functions partially inlined");<br>
>><br>
>> +// Command line option to disable partial-inlining. The default is false:<br>
>> +static cl::opt<bool><br>
>> + DisablePartialInlining("disabl<wbr>e-partial-inlining", cl::init(false),<br>
>> + cl::Hidden, cl::desc("Disable partial<br>
>> ininling"));<br>
>> +<br>
>> +// Command line option to set the maximum number of partial inlining<br>
>> allowed<br>
>> +// for the module. The default value of -1 means no limit.<br>
>> +static cl::opt<int> MaxNumPartialInlining(<br>
>> + "max-partial-inlining", cl::init(-1), cl::Hidden, cl::ZeroOrMore,<br>
>> + cl::desc("Max number of partial inlining. The default is<br>
>> unlimited"));<br>
>> +<br>
>> namespace {<br>
>> struct PartialInlinerImpl {<br>
>> PartialInlinerImpl(InlineFunct<wbr>ionInfo IFI) : IFI(std::move(IFI)) {}<br>
>> @@ -41,6 +52,12 @@ struct PartialInlinerImpl {<br>
>><br>
>> private:<br>
>> InlineFunctionInfo IFI;<br>
>> + int NumPartialInlining = 0;<br>
>> +<br>
>> + bool IsLimitReached() {<br>
>> + return (MaxNumPartialInlining != -1 &&<br>
>> + NumPartialInlining >= MaxNumPartialInlining);<br>
>> + }<br>
>> };<br>
>> struct PartialInlinerLegacyPass : public ModulePass {<br>
>> static char ID; // Pass identification, replacement for typeid<br>
>> @@ -164,6 +181,10 @@ Function *PartialInlinerImpl::unswitchF<wbr>u<br>
>> else<br>
>> llvm_unreachable("All uses must be calls");<br>
>><br>
>> + if (IsLimitReached())<br>
>> + continue;<br>
>> + NumPartialInlining++;<br>
>> +<br>
>> OptimizationRemarkEmitter ORE(CS.getCaller());<br>
>> DebugLoc DLoc = CS.getInstruction()->getDebugL<wbr>oc();<br>
>> BasicBlock *Block = CS.getParent();<br>
>> @@ -185,6 +206,9 @@ Function *PartialInlinerImpl::unswitchF<wbr>u<br>
>> }<br>
>><br>
>> bool PartialInlinerImpl::run(Module &M) {<br>
>> + if (DisablePartialInlining)<br>
>> + return false;<br>
>> +<br>
>> std::vector<Function *> Worklist;<br>
>> Worklist.reserve(M.size());<br>
>> for (Function &F : M)<br>
>><br>
>> Added: llvm/trunk/test/Transforms/Cod<wbr>eExtractor/PartialInlineOptRem<wbr>ark.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeExtractor/PartialInlineOptRemark.ll?rev=301148&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/CodeExtractor/PartialInline<wbr>OptRemark.ll?rev=301148&view=a<wbr>uto</a><br>
>><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/test/Transforms/Cod<wbr>eExtractor/PartialInlineOptRem<wbr>ark.ll<br>
>> (added)<br>
>> +++ llvm/trunk/test/Transforms/Cod<wbr>eExtractor/PartialInlineOptRem<wbr>ark.ll Sun<br>
>> Apr 23 18:39:04 2017<br>
>> @@ -0,0 +1,73 @@<br>
>> +; RUN: opt -S -partial-inliner -pass-remarks=partial-inlining<br>
>> -disable-output < %s 2>&1 | FileCheck %s<br>
>> +; RUN: opt -S -passes=partial-inliner -pass-remarks=partial-inlining<br>
>> -disable-output < %s 2>&1 | FileCheck %s<br>
>> +; RUN: opt -S -partial-inliner -pass-remarks=partial-inlining<br>
>> -disable-output -max-partial-inlining=1 < %s 2>&1 | FileCheck %s<br>
>> +; RUN: opt -S -passes=partial-inliner -pass-remarks=partial-inlining<br>
>> -disable-output -max-partial-inlining=1 < %s 2>&1 | FileCheck %s<br>
>> +<br>
>> +; RUN: opt -S -partial-inliner -pass-remarks=partial-inlining<br>
>> -disable-partial-inlining < %s 2>&1 | FileCheck --check-prefix=LIMIT %s<br>
>> +; RUN: opt -S -passes=partial-inliner -pass-remarks=partial-inlining<br>
>> --disable-partial-inlining < %s 2>&1 | FileCheck --check-prefix=LIMIT %s<br>
>> +; RUN: opt -S -partial-inliner -pass-remarks=partial-inlining<br>
>> -max-partial-inlining=0 < %s 2>&1 | FileCheck --check-prefix=LIMIT %s<br>
>> +; RUN: opt -S -passes=partial-inliner -pass-remarks=partial-inlining<br>
>> -max-partial-inlining=0 < %s 2>&1 | FileCheck --check-prefix=LIMIT %s<br>
>> +<br>
>> +define i32 @bar(i32 %arg) local_unnamed_addr #0 !dbg !5 {<br>
>> +bb:<br>
>> + %tmp = icmp slt i32 %arg, 0, !dbg !7<br>
>> + br i1 %tmp, label %bb1, label %bb2, !dbg !8<br>
>> +<br>
>> +bb1: ; preds = %bb<br>
>> + tail call void (...) @foo() #0, !dbg !9<br>
>> + tail call void (...) @foo() #0, !dbg !10<br>
>> + tail call void (...) @foo() #0, !dbg !11<br>
>> + tail call void (...) @foo() #0, !dbg !12<br>
>> + tail call void (...) @foo() #0, !dbg !13<br>
>> + tail call void (...) @foo() #0, !dbg !14<br>
>> + tail call void (...) @foo() #0, !dbg !15<br>
>> + tail call void (...) @foo() #0, !dbg !16<br>
>> + tail call void (...) @foo() #0, !dbg !17<br>
>> + br label %bb2, !dbg !18<br>
>> +<br>
>> +bb2: ; preds = %bb1, %bb<br>
>> + %tmp3 = phi i32 [ 0, %bb1 ], [ 1, %bb ]<br>
>> + ret i32 %tmp3, !dbg !19<br>
>> +}<br>
>> +<br>
>> +; Function Attrs: nounwind<br>
>> +declare void @foo(...) local_unnamed_addr #0<br>
>> +<br>
>> +; Function Attrs: nounwind<br>
>> +define i32 @dummy_caller(i32 %arg) local_unnamed_addr #0 !dbg !20 {<br>
>> +bb:<br>
>> +; CHECK:remark{{.*}}bar partially inlined into dummy_caller<br>
>> +; LIMIT-NOT:remark{{.*}}bar partially inlined into dummy_caller<br>
>> + %tmp = tail call i32 @bar(i32 %arg), !dbg !21<br>
>> + ret i32 %tmp, !dbg !22<br>
>> +}<br>
>> +<br>
>> +attributes #0 = { nounwind }<br>
>> +<br>
>> +!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
>> +!llvm.module.flags = !{!3}<br>
>> +!llvm.ident = !{!4}<br>
>> +<br>
>> +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer:<br>
>> "clang", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, enums:<br>
>> !2)<br>
>> +!1 = !DIFile(filename: "t.c", directory: "/tmp")<br>
>> +!2 = !{}<br>
>> +!3 = !{i32 2, !"Debug Info Version", i32 3}<br>
>> +!4 = !{!"clang "}<br>
>> +!5 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 3,<br>
>> type: !6, isLocal: false, isDefinition: true, scopeLine: 3, flags:<br>
>> DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)<br>
>> +!6 = !DISubroutineType(types: !2)<br>
>> +!7 = !DILocation(line: 4, column: 14, scope: !5)<br>
>> +!8 = !DILocation(line: 4, column: 6, scope: !5)<br>
>> +!9 = !DILocation(line: 5, column: 5, scope: !5)<br>
>> +!10 = !DILocation(line: 6, column: 5, scope: !5)<br>
>> +!11 = !DILocation(line: 7, column: 5, scope: !5)<br>
>> +!12 = !DILocation(line: 8, column: 5, scope: !5)<br>
>> +!13 = !DILocation(line: 9, column: 5, scope: !5)<br>
>> +!14 = !DILocation(line: 10, column: 5, scope: !5)<br>
>> +!15 = !DILocation(line: 11, column: 5, scope: !5)<br>
>> +!16 = !DILocation(line: 12, column: 5, scope: !5)<br>
>> +!17 = !DILocation(line: 13, column: 5, scope: !5)<br>
>> +!18 = !DILocation(line: 14, column: 5, scope: !5)<br>
>> +!19 = !DILocation(line: 17, column: 1, scope: !5)<br>
>> +!20 = distinct !DISubprogram(name: "dummy_caller", scope: !1, file: !1,<br>
>> line: 19, type: !6, isLocal: false, isDefinition: true, scopeLine: 19,<br>
>> flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)<br>
>> +!21 = !DILocation(line: 21, column: 11, scope: !20)<br>
>> +!22 = !DILocation(line: 21, column: 4, scope: !20)<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
><br>
><br>
><br>
> ______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
><br>
<br>
<br>
<br>
</div></div><span class="m_4244195221455219263m_6142482392848763472HOEnZb"><font color="#888888">--<br>
Davide<br>
<br>
"There are no solved problems; there are only problems that are more<br>
or less solved" -- Henri Poincare<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>