<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Sean, hopefully you’re OK with that reasoning. I went ahead and committed this in r313691.<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 16, 2017, at 10:43 PM, Adam Nemet <<a href="mailto:anemet@apple.com" class="">anemet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Sep 16, 2017, at 4:49 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com" class="">chisophugis@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="auto" class="">Actually maybe something like:<div dir="auto" class=""><br class=""></div><div dir="auto" class="">if (auto &E = ORE.emitMissed(DEBUG_TYPE)) {</div><div dir="auto" class=""> E.emit(...) << ...;</div><div dir="auto" class="">}</div></div></div></blockquote><div class=""><br class=""></div><div class="">Well, the point of this interface was exactly to avoid writing a conditional. If you’re willing to use a conditional you can already write this:</div><div class=""><br class=""></div><div class="">if (ORE.allowExtraAnalysis(DEBUG_TYPE))</div><div class=""> ORE.emit(OptimizationRemark(…) << …;</div><div class=""><br class=""></div><div class="">But again the point was to hide all this. I find the closure-based interface more concise and easier to identify visually. One reason is that the block is contained within ORE.emit(…). I just wish you could omit the return in a lambda as in Python.</div><div class=""><br class=""></div><div class="">Adam</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="auto" class=""><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Credit to Chandler for that (if I'm remembering it right. This is vaguely recollected from a LLVM social conversation a long time ago about reducing overhead of clang diagnostics that are not enabled, which sounds like the same problem)</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Sep 16, 2017 4:39 PM, "Sean Silva" <<a href="mailto:chisophugis@gmail.com" class="">chisophugis@gmail.com</a>> wrote:<br type="attribution" class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto" class=""><div dir="auto" class="">Another alternative could be:</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">ORE.emitMissed(DEBUG_TYPE, ...) << ...</div><div dir="auto" class=""><br class=""></div><div dir="auto" class="">Then the first line of emitMissed does a check if it is enabled and if not then returns a dummy stream that does nothing for operator<< (and short-circuits all the stream operations)</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Sep 15, 2017 2:21 PM, "Adam Nemet via llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a>> wrote:<br type="attribution" class=""><blockquote class="m_-4736550687993114039quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">For better readability we typically create remarks and call OptimizationRemarkEmitter::emi<wbr class="">t unconditionally. E.g.:<div class=""><div class=""><br class=""></div><div class=""><div class="">Transforms/IPO/Inliner.cpp: ORE.emit(OptimizationRemarkMi<wbr class="">ssed(DEBUG_TYPE, "TooCostly", Call)</div><div class="">Transforms/IPO/Inliner.cpp- << NV("Callee", Callee) << " not inlined into "</div><div class="">Transforms/IPO/Inliner.cpp- << NV("Caller", Caller) << " because too costly to inline (cost="</div><div class="">Transforms/IPO/Inliner.cpp- << NV("Cost", IC.getCost())</div><div class="">Transforms/IPO/Inliner.cpp- << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")");</div></div><div class=""><br class=""></div><div class="">Then inside ORE we return right away if the remarks for the given pass is not enabled.<div class=""><br class=""></div><div class="">This is nice and concise however there is still some overhead involved in this if remarks are not enabled:</div><div class=""><br class=""></div><div class="">1. Constructing the remark object</div><div class="">2. Computing and inserting the strings, named-value arguments</div></div><div class="">3. Performing the comparison between the pass name and the passes enabled by the user</div><div class="">4. Virtual destructor</div><div class=""><br class=""></div><div class="">Now that Vivek added support for asking LLVMContext for what remarks are enabled by the user [1] we can improve this. The idea is to allow ORE::emit to take a closure. This delays all this work until we know remarks are enabled. E.g. the above code with closure:</div><div class=""><br class=""></div><div class=""><div class=""> ORE.emit(<b class="">[&]() {</b></div><div class=""> <b class="">return</b> OptimizationRemarkMissed(DEBUG<wbr class="">_TYPE, "TooCostly", Call)</div><div class=""> << NV("Callee", Callee) << " not inlined into "</div><div class=""> << NV("Caller", Caller) << " because too costly to inline (cost="</div><div class=""> << NV("Cost", IC.getCost())</div><div class=""> << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")"<b class="">;</b></div><div class=""><b class=""> }</b>);</div></div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">I have a proof-of-concept implementation at <a href="https://reviews.llvm.org/D37921" target="_blank" class="">https://reviews.llvm.org/D3<wbr class="">7921</a>.</div><div class=""> </div><div class="">The main change is that since in the lambda the remark is now returned by value, we need to preserve its type in the insertion operator. This requires making the insertion operator generic. I am also curious if people see C++ portability problems with the code.</div><div class=""><br class=""></div><div class="">Feedback welcome.</div><div class=""><br class=""></div><div class="">Adam</div><div class=""><br class=""></div><div class="">[1] <a href="https://reviews.llvm.org/D33514" target="_blank" class="">https://reviews.llvm.org/D<wbr class="">33514</a></div></div><div class=""><br class=""></div></div><br class="">______________________________<wbr class="">_________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank" class="">llvm-dev@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-dev</a><br class="">
<br class=""></blockquote></div><br class=""></div>
</blockquote></div></div>
</div></blockquote></div><br class=""></div></div></blockquote></div><br class=""></div></body></html>