<p dir="ltr">This is different from my patch, right? For linkoncelinage and comdat and address taken, we do want to record the address.</p>
<p dir="ltr">This version will not, however.</p>
<p dir="ltr">Rong</p>
<div class="gmail_quote">On Apr 26, 2016 12:19 PM, "Betul Buyukkurt" <<a href="mailto:betulb@codeaurora.org">betulb@codeaurora.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">betulb added inline comments.<br>
<div class="quoted-text"><br>
================<br>
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:241<br>
@@ -236,2 +240,3 @@<br>
</div><div class="quoted-text">+    return false;<br>
   // Check the linkage<br>
   if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&<br>
</div>----------------<br>
<div class="quoted-text">xur wrote:<br>
> betulb wrote:<br>
> > I do not know yet much about COMDAT's. I need to read about them. Otherwise, the check is fine. But, I'd prefer it to be placed after line 244. There we reduce the linkages to check for to one of linkonce, local or available externally.  I'd also like to check if the same warning shows up for either of linkonce or available externally linkages.<br>
> I have no problem moving it down since these two checks are exclusive for now.<br>
> But I'm wondering why your prefer the different order.<br>
> To me,  "return false" check should ahead "return true" as this is a correctness issue.<br>
><br>
> other linkages do not seem to matter here.<br>
</div>Thanks. My thought was had the other linkages mattered, the COMDAT check could have been merged into the final return statement as:<br>
<br>
return  !F->hasComdat() && F->hasAddressTaken();<br>
<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19515" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19515</a><br>
<br>
<br>
<br>
</blockquote></div>