<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 31, 2014 at 9:27 AM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On 3/30/2014 11:11 PM, Rui Ueyama wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is not an obvious change to Core, not suitable for post-commit review.<br>
Please roll it back until we reach a conclusion that this is the right<br>
design.<br>
<br>
</blockquote></div>
I am not sure why its not a change, that has to be present in core. See my notes below.</blockquote><div><br></div><div>I don't get it. What do you mean?</div><div><br></div><div>Anyways, 1) this change is to a common component of LLD shared by all ports, 2) made everybody writing code for COMDAT to check gnu.linkonce too, and 3) was not discussed if that is the right design. If this repository is owned by you that's fine but this is a shared repository. Please follow the rule.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
==============================<u></u>==============================<u></u>==================<br>
--- lld/trunk/include/lld/Core/<u></u>DefinedAtom.h (original)<br>
+++ lld/trunk/include/lld/Core/<u></u>DefinedAtom.h Sun Mar 30 22:16:37 2014<br>
@@ -147,6 +147,7 @@ public:<br>
      typeRWNote,             // Identifies readwrite note sections [ELF]<br>
      typeNoAlloc,            // Identifies non allocatable sections [ELF]<br>
      typeGroupComdat,        // Identifies a section group [ELF, COFF]<br>
+    typeGnuLinkOnce,        // Identifies a gnu.linkonce section [ELF]<br>
<br>
</blockquote>
So you defined a new type for gnu.linkonce sections, but throughout this<br>
patch typeGnuLinkOnce is not distinguished from typeGroupComdat.<br>
</blockquote></div>
There is one thing that distinguishes between comdat and linkonce in the patch, see my below comment.<div class=""><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  All if's<br>
previously handled typeGroupComdat are now replaced with typeGroupComdat ||<br>
typeGnuLinkOnce. That does not look good.<br>
<br>
I don't see the reason we can't simply treat gnu.linkonce as<br>
typeGroupComdat. If we treat gnu.linkonce such a way, only the ELF object<br>
reader will have to handle gnu.linkonce sections and Resolver will remain<br>
unchanged.<br>
</blockquote></div>
The only distinguishing factor with gnu linkonce and group comdat is to produce an error when the same group signature is part of COMDAT and the same group has a signature with linkonce.<br>
<br>
I dont think we can use typeGroupComdat for linkonce and comdat.<br>
<br>
Thanks<span class="HOEnZb"><font color="#888888"><br>
<br>
Shankar Easwaran<br>
</font></span></blockquote></div><br></div></div>