<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><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><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. I/other contributors may not be so skilled to do something really big cool here :), yet.</p><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><p 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.</p></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">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>