<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 22, 2014 at 4:44 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sorry for being late on my own thread. Trying to reply to most comments at once:<br>
<br>
* I am pretty sure that we want to split the "in a comdat" notion from<br>
the linkage. I am not married to how this patch does it.<br>
* On MachO. This should actually improve things. A plain linkonce_odr<br>
will then mean the same thing on ELF and MachO and a comdat in MachO<br>
gets rejected early.<br>
* One Idea I had was having a selfcomdat syntactic sugar in the .ll,<br>
but I think I like rnk's suggestion better: just omit the $value (and<br>
flipping the order in the case of globals).<br>
* For the idea of using external instead of linkonce linkage. I agree<br>
with David. We should be making these things independent. Keeping the<br>
linkonce_odr/weak_odr is useful for the 'odr' bit, for saying the<br>
symbol should be STB_WEAK and for the can/cannot be dropped<br>
distinction.<br></blockquote><div><br></div><div>Yes, I agree these things should be independent. (Note that I think that STB_WEAK and can/cannot be dropped are orthogonal -- an inline function definition is discardable, but does not fundamentally need to be STB_WEAK if we can put it in a comdat.)</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* I like the idea of optimizing the case of a select any that has the<br>
same name as an existing symbol, but it is not that simple in cases<br>
like<br>
<br>
$v = comdat any<br>
@v = global i32 42 comdat $v<br>
@bar = global i32 42 comdat $v<br>
<br>
When we are building bar, how do we find if there is a "simple comdat<br>
$v"? We would have to first lookup a GV named $v. When building @bar<br>
first and @v second we would still need to look at the "normal comdat<br>
table" when building $v to see if a comdat was already there.<br>
<br>
Since the main saving is on the symbol names, another idea would be to replace<br>
<br>
typedef StringMap<Comdat> ComdatSymTabType;<br>
<br>
With<br>
<br>
typedef DenseMap<const char *, Comdat> ComdatSymTabType;<br>
<br>
And have an extra StringMap with the non-symbol names like C5/D5 (or<br>
even introduce name -> null mappings in the gv symbol table?).<br>
<br>
But it would have the main constraint as the full optimization: when<br>
dropping the comdat, We have to be sure to drop $v last or we might<br>
end up we with a pointer to garbage.<br>
<br>
So, how about splitting work in:<br>
<br>
* llvm: Implement the syntatic change for comdats with the same name<br>
as the GV, but keep the current representation.<br>
* clang: this patch, but with less test change noise.<br>
* llvm: stop producing implicit comdats.<br>
* llvm: implement and benchmark one of the possible optimizations.</blockquote><div><br></div><div>This sounds like a good path forward to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
On 20 December 2014 at 19:13, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:<br>
><br>
><br>
> On Fri, Dec 19, 2014 at 5:43 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> wrote:<br>
>><br>
>> On Fri, Dec 19, 2014 at 12:51 PM, Rafael Espíndola<br>
>> <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>>><br>
>>> There is quiet a bit of history behind this.<br>
>>><br>
>>> The llvm IR until very recently had no support for comdats. This was a<br>
>>> problem when targeting C++ on ELF/COFF as just using weak linkage<br>
>>> would cause quiet a bit of dead bits to remain on the executable<br>
>>> (unless -ffunction-sections, -fdata-sections and --gc-sections were<br>
>>> used).<br>
>>><br>
>>> To fix the problem, llvm's codegen will just assume that any weak or<br>
>>> linkonce that is not in an explicit comdat should be output in one<br>
>>> with the same name as the global.<br>
>>><br>
>>> This unfortunately breaks cases like pr19848 where a weak symbol is<br>
>>> not expected to be part of any comdat.<br>
>>><br>
>>> Now that we have explicit comdats in the IR, we can finally get both<br>
>>> cases right.<br>
>>><br>
>>> This first patch just makes clang give explicit comdats to<br>
>>> GlobalValues where it is allowed to.<br>
>>><br>
>>> A followup patch to llvm will then stop implicitly producing comdats.<br>
>><br>
>><br>
>> I'm not sure this is the right direction, but if it is, this change looks<br>
>> reasonable to me.<br>
><br>
><br>
> I think this patch is fine, what are your reservations?<br>
><br>
>><br>
>><br>
>> Assuming we want to go in this direction, I think we should also consider<br>
>> switching from a weak linkage to a strong linkage for entities governed by<br>
>> the ODR; using weak linkage is a hack, since these symbols are really not<br>
>> weak in the usual sense.<br>
><br>
><br>
> Maybe but I think that can be considered separately from this change.<br>
><br>
>><br>
>> That exposes a hole in our comdat support: we should have the ability to<br>
>> define a discardable comdat (which need not be emitted if none of its<br>
>> symbols are used in the current TU); a discardable comdat with an external<br>
>> function definition seems like the right representation for an inline<br>
>> function.<br>
><br>
><br>
> I don't think that we want to abuse linkage like that.  We really need to<br>
> sit down untangle this mess.  Before comdats, there was never a way to say<br>
> "give me a global, stick it in a COMDAT but *don't* make it weak",<br>
> COMDAT-ness and weak-ness came hand in hand.<br>
><br>
> Now we have COMDATs and the world of linkage is unchanged, I don't think<br>
> that this is a good place to be.<br>
><br>
> We really need to kill linkage and replace it with several things, each of<br>
> which individually control the behavior of the global.  Things like<br>
> @llvm.used are a hack to work around the lack of flexibility linkage has to<br>
> offer.<br>
><br>
> I'm more than happy to help out with this if we can get consensus on a<br>
> design.<br>
</div></div></blockquote></div><br></div></div>