[PATCH] D19515: [PGO] Prohibit the recording the function address if it's internal and COMDAT.

Rong Xu via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 12:53:04 PDT 2016


This is different from my patch, right? For linkoncelinage and comdat and
address taken, we do want to record the address.

This version will not, however.

Rong
On Apr 26, 2016 12:19 PM, "Betul Buyukkurt" <betulb at codeaurora.org> wrote:

betulb added inline comments.

================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:241
@@ -236,2 +240,3 @@
+    return false;
   // Check the linkage
   if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&
----------------
xur wrote:
> betulb wrote:
> > 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.
> I have no problem moving it down since these two checks are exclusive for
now.
> But I'm wondering why your prefer the different order.
> To me,  "return false" check should ahead "return true" as this is a
correctness issue.
>
> other linkages do not seem to matter here.
Thanks. My thought was had the other linkages mattered, the COMDAT check
could have been merged into the final return statement as:

return  !F->hasComdat() && F->hasAddressTaken();



http://reviews.llvm.org/D19515
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160426/993e66d5/attachment.html>


More information about the llvm-commits mailing list