<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 1/4/19 6:46 AM, Clement Courbet wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAHOnJtpOhLd7de_A+sBQc6Ofs+DLfSMZ9GndSa4yMbFp29K_Hw@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>> I don't think the function attribute works here because we want this to be globally enabled instead of per-function (but maybe I misunderstood what you were suggesting).<br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Our general scheme for these kinds of things is to add a per-function attribute, and then have the frontend add the attribute to every function in the module. The rationale for this scheme comes from how it interacts with the separate-compilation/LTO model:
Imagine I have some flag affecting code generation, say -fenable-memeq, and I use that flag when compiling some source files and not others, and then I use LTO, I want the option to apply only to code in those functions which came from translation units compiled
with the -fenable-memeq flag.</p>
<p>That having been said, I see several reasons to favor the per-call-site attribute over the function-level attribute in this case:</p>
<p> 1. With function-level attributes, there's always the question of what to do with inlining. Either you block inlining upon an attribute mismatch, or you allow it and drop the conflicting attributes in some conservative sense. With call-site attributes,
this is not an issue.</p>
<p> 2. The attribute will apply rarely, and so while putting the attribute on all functions will increase the bitcode size / memory footprint in the common case, having it only on relevant call sites will not have that overhead.</p>
<p> 3. While we have one function now, there could be a large number, and encoding these all in function-level attributes on every function will become unwieldy (because it will magnify the issues above). Having the frontend attach information per-call-site
seems like a better approach.</p>
<p>To bikeshed, "trap-func-name", why "trap"?<br>
</p>
<p> -Hal</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAHOnJtpOhLd7de_A+sBQc6Ofs+DLfSMZ9GndSa4yMbFp29K_Hw@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>Hm, actually I think you might have been referring to a function attribute at call site, same as `trap-func-name` in `CodeGenModule::ConstructDefaultFnAttrList`:</div>
<div><br>
</div>
<div>
<div>if (AttrOnCallSite) {</div>
<div> // Attributes that should go on the call site only.</div>
<div> if (!CodeGenOpts.SimplifyLibCalls ||</div>
<div> CodeGenOpts.isNoBuiltinFunc(Name.data()))</div>
<div> FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);</div>
<div> if (!CodeGenOpts.TrapFuncName.empty())</div>
<div> FuncAttrs.addAttribute("trap-func-name", CodeGenOpts.TrapFuncName);</div>
<div> }</div>
</div>
<div><br>
</div>
<div>The nice thing is that `trap-func-name` paves the way as it's a pretty similar use case. Here's a v3 diff with this approach for comparison: <a href="https://reviews.llvm.org/D56313" moz-do-not-send="true">https://reviews.llvm.org/D56313</a>.</div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Fri, Jan 4, 2019 at 11:27 AM Clement Courbet <<a href="mailto:courbet@google.com" moz-do-not-send="true">courbet@google.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div>Thanks for the suggestions Hal,</div>
<div><br>
</div>
<div>So if I understand correctly, you're recommending we add a <a href="https://llvm.org/docs/LangRef.html#module-flags-metadata" target="_blank" moz-do-not-send="true">module flag</a> to LLVM, something like:</div>
<div><br>
</div>
<div>
<div>!llvm.module.flags = !{..., !123}</div>
<div>!123 = !{i32 1, !"memeq_lib_function", !"user_memeq"}</div>
</div>
<div><br>
</div>
<div>I've given it a try in the following patch: <a href="https://reviews.llvm.org/D56311" target="_blank" moz-do-not-send="true">https://reviews.llvm.org/D56311</a></div>
<div>If this sounds reasonable I can start working on adding a CodeGenOptions to clang to see what this entails.</div>
<div><br>
</div>
<div>I don't think the function attribute works here because we want this to be globally enabled instead of per-function (but maybe I misunderstood what you were suggesting).</div>
<br class="gmail-m_3137235587726084387gmail-Apple-interchange-newline">
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Jan 3, 2019 at 6:40 PM Finkel, Hal J. <<a href="mailto:hfinkel@anl.gov" target="_blank" moz-do-not-send="true">hfinkel@anl.gov</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><br>
<div class="gmail-m_3137235587726084387gmail-m_-276203305088383900moz-cite-prefix">
On 1/3/19 3:29 AM, Clement Courbet via llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>
<div>Hi all,</div>
<div><br>
</div>
<div>We'd like to suggest <b>adding a -memeq-lib-function</b> flag to allow the user to specify a `<b>memeq()</b>` function to improve string equality check performance.
<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<p>Hi, Clement,</p>
<p>We really shouldn't be adding backend flags for anything at this point (except for debugging and the like). A function attribute should be fine, or global metadata if necessary. A function attribute should play better with LTO, and so that's generally the
recommended design point.</p>
<p><br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>
<div><br>
</div>
<div>Right now, when llvm encounters a <b>string equality check</b>, e.g. `if (memcmp(a, b, s) == 0)`, it tries to expand to an equality comparison if `s` is a small compile-time constant, and falls back on calling `memcmp()` else.</div>
<div><br>
</div>
<div>This is sub-optimal because memcmp has to compute much more than equality.</div>
<div> </div>
<div>We propose adding a way for the user to specify a `memeq` library function (e.g. `-memeq-lib-function=user_memeq`) which will be called instead of `memcmp()` when the result of the memcmp call is only used for equality comparison.</div>
<div><br>
</div>
<div>`memeq` can be made much more efficient than `memcmp` because equality comparison is trivially parallel while lexicographic ordering has a chain dependency.</div>
</div>
<div><br>
</div>
<div>We measured an very large improvement of this approach on our internal codebase. A significant portion of this improvement comes from the stl, typically `std::string::operator==()`.</div>
<div><br>
</div>
<div>Note that this is a <b>backend-only change</b>. Because the c family of languages do not have a standard `memeq()` (posix used to have `bcmp()` but it was removed in 2001), c/c++ code cannot communicate the equality comparison semantics to the compiler.</div>
<div><br>
</div>
<div>We did not add an RTLIB entry for memeq because the user environment is not guaranteed to contain a `memeq()` function as the libc has no such concept.</div>
<div><br>
</div>
<div>If there is interest, we could also contribute our optimized `memeq` to compiler-rt.</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<p>That would be useful.<br>
</p>
<p>Thanks again,</p>
<p>Hal</p>
<p><br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>
<div>A proof of concept patch for this for this RFC can be found here: <a href="https://reviews.llvm.org/D56248" target="_blank" moz-do-not-send="true">https://reviews.llvm.org/D56248</a></div>
</div>
<div><br>
</div>
<div>Comments & suggestions welcome !</div>
<div>Thanks,</div>
<div><br>
</div>
<div>Clement</div>
</div>
</div>
</div>
</div>
<br>
<fieldset class="gmail-m_3137235587726084387gmail-m_-276203305088383900mimeAttachmentHeader">
</fieldset>
<pre class="gmail-m_3137235587726084387gmail-m_-276203305088383900moz-quote-pre">_______________________________________________
LLVM Developers mailing list
<a class="gmail-m_3137235587726084387gmail-m_-276203305088383900moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">llvm-dev@lists.llvm.org</a>
<a class="gmail-m_3137235587726084387gmail-m_-276203305088383900moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<pre class="gmail-m_3137235587726084387gmail-m_-276203305088383900moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</blockquote>
</div>
</blockquote>
</div>
</blockquote>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>