<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 17, 2014 at 1:32 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Wed, Dec 17, 2014 at 12:48 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Are we sure this is correct? In the motivating example, what if 'f' gets inlined? Then 'g' will have the code of 'f' embedded into it, but it will not be part of the comdat that statically initializes the guard variable.<div><br></div><div><div><span><div>struct S {</div><div>  static const int x;</div><div>};</div></span><div>// const int S::x = 42;</div><span><div>inline const int *f() {</div><div>  static const int y = S::x;</div><div>  return &y;</div><div>}</div><div>const int *g() { return f(); }</div></span></div><div><br></div></div><div>I spent a coffee break a few weeks ago worrying about this, and we came to the conclusion that we can't do this if we want to support inlining from a comdat. Maybe it's illegal for LLVM to inline f from a comdat, but that's pretty sad.</div></div></blockquote><div><br></div></span><div>It is. Do we have any alternative that supports inlining and the above testcase?</div></div></div></div></blockquote><div><br></div><div>(Summarizing offline discussion)</div><div><br></div><div>While evaluating the initializer for a static local variable, we can track whether the initialization depends on any properties that might not hold in another translation unit. If it does, then we should assume that the variable may have dynamic initialization in another TU. In that case:</div><div><br></div><div> - generate a guard variable that is initialized to the "already initialized" state.</div><div> - generate code to initialize the variable within the local function (just copy across the constant value).<br></div><div><br></div><div>This removes any need to put the inline function into a COMDAT with the static local variable. (We still need to put the local variable and its guard variable in a COMDAT together, though.)</div><div><br></div><div><br>Separately, it seems that LLVM should not inline across COMDAT boundaries, unless it somehow knows that every definition of the COMDAT will provide an equivalent definition of the symbol (for instance, because it's got an *_odr linkage), and likewise for other optimizations that transfer information between globals. A strong definition in a COMDAT should behave similarly to a weak definition.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Tue, Dec 16, 2014 at 1:02 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On 1 November 2014 at 14:29, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
> Thanks, that sounds ideal.<br>
<br>
Finally fixed by r224369. Sorry for the delay.<br>
</div></div><div><div><br>
Cheers,<br>
Rafael<span><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</span></div></div></blockquote></div></div>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>