<div dir="ltr"><div>Typical case is</div><div><br></div>void log(data) {<div> file = fopen(....)</div><div> for (...) </div><div> fwrite(data ..)</div><div> ... maybe some more logging ...</div><div> close(file);</div><div>} // cond: file does not escape</div><div><br></div><div>Here we replace e.g. fwrite (in this example) (which needs to lock and unlock internal lock to be thread safe) with fwrite_unlocked which does not expensive lock/unlock. </div><div>I tried several small open source projects and I got hits.</div><div><br></div><div>I can make it off by default and provide option to enable it. Any ideas for option name?</div><div><br></div><div>- What does the typical improvement look like? Avoid useless locking in I/O functions, faster output to files.<br></div><div>- What do you think the policy should be for simplifying libcalls in a way that can break interceptors? </div><div>-> off by default + option to enable. Maybe interceptors can be extended in the future to handle this transformation/function calls?</div></div><br><div class="gmail_quote"><div dir="ltr">ut 24. 7. 2018 o 19:29 Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> napísal(a):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div>Was this ever fully resolved?</div><div><br></div><div>Folks on this thread were interested in finding out more about the cost/benefit tradeoff of doing this optimization. I'm not sure that's been adequately explored.</div><div><br></div><br><blockquote type="cite">On May 31, 2018, at 12:34 PM, Dávid Bolvanský via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></blockquote><div><blockquote type="cite"><br class="m_-8690793124328199460Apple-interchange-newline"><div><div dir="auto"><p style="margin:0px 0px 12px;padding:0px;border:0px;font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">In my small C tests and with some file io-related open source programs I see some small improvements - yes, nothing "wow" - but still worth for me to try to do it in LLVM.</p></div></div></blockquote>I'd like to see a fuller justification here. Could you elaborate?</div><div><br></div><div>- What does the typical improvement look like?</div><div><br></div><div>- How common is this pattern is in the open source programs you looked at?</div><div><br></div><div>- What do you think the policy should be for simplifying libcalls in a way that can break interceptors?<br><div><br></div><div>Could you CC the reviewer, rja (Joe) on this thread? They may have some insight. I can't find their email address. They appear to have stopped commenting on Phab shortly after this commit landed.</div><div><br></div><br><blockquote type="cite"><div><div dir="auto"><p style="margin:0px 0px 12px;padding:0px;border:0px;font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">I am not sure about the possible rule "if no interesting numbers in SPEC, do not do it". I still consider this as a quite interesting transformation. And I learnt a lot during this work.</p></div></div></blockquote><div>It's great that you find this work interesting and educational, but that alone isn't a sufficient justification for adding an optimization.</div><div><br></div><br><blockquote type="cite"><div><div dir="auto"><p style="margin:0px 0px 12px;padding:0px;border:0px;font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">We could remove the whole SimplifyLibcalls since it brings micro optimizations. Sorry to say, but then should be a warning: don't work on small/micro optimizations, we dont like/want it.</p></div></div></blockquote><div>There may be a high bar for clearing certain changes, but I'd like to think that the llvm community is generally welcoming :). Equating concern over one change to a removal of a chunk of the compiler is a false comparison.</div><div><br></div><br><blockquote type="cite"><div><div dir="auto"><p style="margin:0px 0px 12px;padding:0px;border:0px;font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">If you have list of things which are "todo/we would like to focus on them" for LLVM, I would like to see them. So maybe I could work on something less controversal (vs. this patch) :)</p></div></div></blockquote>Some of the projects here are actively being worked on for GSoC, but I'm sure any number of them could use more help: <a href="http://llvm.org/OpenProjects.html" target="_blank">http://llvm.org/OpenProjects.html</a>.</div><div><br></div><div><br></div><div><blockquote type="cite"><div><div dir="auto"><div style="margin:0px;padding:0px;border:0px;font-family:"segoe ui","segoe ui emoji","segoe ui symbol",lato,"helvetica neue",helvetica,arial,sans-serif;font-size:13px;background-color:rgb(255,255,255)">tl;dr: I am fine if a there should be a new option to enable it (default: off) or if you decide to fully remove it.</div></div></div></blockquote><div><br></div><div>I think it would be fine to keep this change in tree provided that we get a better understanding of the tradeoffs here.</div><div><br></div><div>thanks!</div><div>vedant</div><div><br></div><br><blockquote type="cite"><div><br><div class="gmail_quote"><div dir="ltr">Dňa št 31. 5. 2018, 21:00 Friedman, Eli <<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>> napísal(a):<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 5/31/2018 12:07 AM, Eric Christopher wrote:<br>
> I'm currently not entirely sure we want to do this class of <br>
> optimizations for a few reasons:<br>
><br>
> a) This ends up being an interesting educational problem on the <br>
> contents of a particular platform's libc - in particular, people that <br>
> write small libc interceptors are going to have to update just for <br>
> this in order to successfully build. I updated a few recently, but <br>
> this seems to be something that's going to be frustrating at llvm <br>
> release time. It will require anyone that's using a libc interceptor <br>
> to run into some odd problems if they don't -fno-builtin some new <br>
> functions or make sure that they add the unlocked versions which are <br>
> technically not required, but may exist on a system.<br>
<br>
Yes, this is a concern. But I'm not sure where exactly we would draw <br>
the line here; do we want to document some "minimal" set of C library <br>
functions that LLVM might call for each target, given input which <br>
doesn't explicitly call those functions? Or are functions that do I/O <br>
special? Or do we just want to use some informal "don't rock the boat <br>
unless we see improvements on SPEC" rule? (We already do various <br>
similar transforms, like printf->puts, sin->sinf, automatic recognition <br>
of memset_pattern16, etc.)<br>
<br>
> b) I believe I've found a bug in the current (theoretical) basis of <br>
> this which is: fopen could capture the file argument and do whatever <br>
> it wants with it. (and even after my fix, -fno-builtin-fopen still <br>
> didn't have individual -fno-builtin support which means you really <br>
> couldn't distinguish this at the time from clang - I've since fixed <br>
> this). This may not be the case anymore, but...<br>
<br>
Assuming fopen returns a unique FILE* is just like assuming malloc <br>
returns a unique memory allocation; it's just assuming the C library <br>
works as documented. This doesn't seem particularly controversial.<br>
<br>
> What's the overall gain we're getting for doing this optimization? It <br>
> seems to be a lot of pain for not a lot of win in general <br>
> applications? Is there some benchmark here?<br>
<br>
Dávid said he saw substantial gains on microbenchmarks... not sure what <br>
sort of general benchmarking was involved. I'll let him comment on that <br>
more.<br>
<br>
-Eli<br>
<br>
-- <br>
Employee of Qualcomm Innovation Center, Inc.<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br>
<br>
</blockquote></div>
_______________________________________________<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" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></blockquote></div>