[llvm] r332452 - [SimplifyLibcalls] Replace locked IO with unlocked IO

Wed Jul 25 10:58:35 PDT 2018

Typical case is

void log(data) {
      file = fopen(....)
      for (...)
          fwrite(data ..)
     ... maybe some more logging ...
     close(file);
} // cond: file does not escape

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.
I tried several small open source projects and I got hits.

I can make it off by default and provide option to enable it. Any ideas for
option name?

- What does the typical improvement look like? Avoid useless locking in I/O
functions, faster output to files.
- What do you think the policy should be for simplifying libcalls in a way
that can break interceptors?
-> off by default + option to enable. Maybe interceptors can be extended in
the future to handle this transformation/function calls?

ut 24. 7. 2018 o 19:29 Vedant Kumar <vsk at apple.com> napísal(a):

> Was this ever fully resolved?
>
> 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.
>
>
> On May 31, 2018, at 12:34 PM, Dávid Bolvanský via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>
> 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.
>
> I'd like to see a fuller justification here. Could you elaborate?
>
> - What does the typical improvement look like?
>
> - How common is this pattern is in the open source programs you looked at?
>
> - What do you think the policy should be for simplifying libcalls in a way
> that can break interceptors?
>
> 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.
>
>
> 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.
>
> It's great that you find this work interesting and educational, but that
> alone isn't a sufficient justification for adding an optimization.
>
>
> 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.
>
> 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.
>
>
> 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) :)
>
> Some of the projects here are actively being worked on for GSoC, but I'm
> sure any number of them could use more help:
> http://llvm.org/OpenProjects.html.
>
>
> 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.
>
>
> I think it would be fine to keep this change in tree provided that we get
> a better understanding of the tradeoffs here.
>
> thanks!
> vedant
>
>
>
> Dňa št 31. 5. 2018, 21:00 Friedman, Eli <efriedma at codeaurora.org>
> napísal(a):
>
>> On 5/31/2018 12:07 AM, Eric Christopher wrote:
>> > I'm currently not entirely sure we want to do this class of
>> > optimizations for a few reasons:
>> >
>> > a) This ends up being an interesting educational problem on the
>> > contents of a particular platform's libc - in particular, people that
>> > write small libc interceptors are going to have to update just for
>> > this in order to successfully build. I updated a few recently, but
>> > this seems to be something that's going to be frustrating at llvm
>> > release time. It will require anyone that's using a libc interceptor
>> > to run into some odd problems if they don't -fno-builtin some new
>> > functions or make sure that they add the unlocked versions which are
>> > technically not required, but may exist on a system.
>>
>> Yes, this is a concern.  But I'm not sure where exactly we would draw
>> the line here; do we want to document some "minimal" set of C library
>> functions that LLVM might call for each target, given input which
>> doesn't explicitly call those functions?  Or are functions that do I/O
>> special?  Or do we just want to use some informal "don't rock the boat
>> unless we see improvements on SPEC" rule?  (We already do various
>> similar transforms, like printf->puts, sin->sinf, automatic recognition
>> of memset_pattern16, etc.)
>>
>> > b) I believe I've found a bug in the current (theoretical) basis of
>> > this which is: fopen could capture the file argument and do whatever
>> > it wants with it. (and even after my fix, -fno-builtin-fopen still
>> > didn't have individual -fno-builtin support which means you really
>> > couldn't distinguish this at the time from clang - I've since fixed
>> > this). This may not be the case anymore, but...
>>
>> Assuming fopen returns a unique FILE* is just like assuming malloc
>> returns a unique memory allocation; it's just assuming the C library
>> works as documented.  This doesn't seem particularly controversial.
>>
>> > What's the overall gain we're getting for doing this optimization? It
>> > seems to be a lot of pain for not a lot of win in general
>> > applications? Is there some benchmark here?
>>
>> Dávid said he saw substantial gains on microbenchmarks... not sure what
>> sort of general benchmarking was involved.  I'll let him comment on that
>> more.
>>
>> -Eli
>>
>> --
>> Employee of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>> Linux Foundation Collaborative Project
>>
>> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180725/63c3574d/attachment.html>