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

[Betul Buyukkurt via llvm-commits]

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