<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Nov 13, 2017 at 11:25 AM Friedman, Eli <<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 11/10/2017 7:54 PM, Chandler Carruth via llvm-dev wrote:<br>
> Trying to sum-up the approaches that have been discussed, numbered in<br>
> the order I saw them:<br>
><br>
> 1) Mangle internal names to avoid collisions.<br>
><br>
> 2) Only optimize library functions when they have external linkage.<br>
><br>
> 3) Switch optimizations to do cloning rather than mutating functions<br>
><br>
> 4) Mark all library functions declared in system headers with some<br>
> attribute and key optimizations on this<br>
><br>
><br>
> #1 doesn't seem to have much appeal.<br>
> #3 is interesting and likely a good thing to do but not really<br>
> sufficient to fix the root issue.<br>
> #4, especially in the mode w here these attributes actually carry the<br>
> semantics allowing the name-based heuristics to be isolated in a more<br>
> appropriate layer, seems like a very interesting long term path, but<br>
> honestly not one I have the time to bring about right now. And I don't<br>
> think we can wait for this to fix things.<br>
><br>
> But I think we can combine some of #4 and some of #2 to get a good<br>
> solution here that is practical and achievable:<br>
><br>
> - Recognize external library functions, much like we already do, but<br>
> restrict it to external functions.<br>
> - Recognize internal functions *with a builtin attribute* much like we<br>
> do external library functions.<br>
> - Teach internalize to add the builtin attribute as it changes linkage.<br>
><br>
> One example of what I *really* want from this even in LTO which<br>
> motivates the change to internalize: things like 'readonly' where some<br>
> spec lets us optimize callers with this even if the implementation<br>
> actually writes to memory. Consider building with -fno-math-errno and<br>
> LTOing a libc that does actually set errno in its implementation.<br>
<br>
If we're LTO'ing in an entire libc, there are certain functions which<br>
are special in weird ways, which I'm not sure we can represent properly<br>
with your suggested representation.  ISel can generate calls to C<br>
library functions (everything in RuntimeLibcalls.def, including<br>
memcpy/memset/memmove and a bunch of libm functions).  And the "noalias"<br>
attribute on malloc() depends on the fact that we can't actually see the<br>
implementation normally.  But maybe we can work on that incrementally?<br></blockquote><div><br></div><div>Yeah, there are a lot of issues that start to crop up here. But my hope is exactly what you say: we should handle that incrementally.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Also, as a side-note, -fno-math-errno isn't really a great example. The<br>
fact that we add readnone to math functions which can set errno is a<br>
bug; we just haven't fixed it because speed is more important than<br>
correctness in fast-math mode, and nobody has implemented a suitable<br>
alternative.  (It's easy to write a testcase where we miscompile because<br>
a math function clobbers the errno set by some other function.)<br></blockquote><div><br></div><div>Sure, but I think it makes the point.</div></div></div>