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

[Vedant Kumar via llvm-commits]

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 <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 <mailto: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/20180724/97aec47a/attachment.html>