[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 11:39:40 PDT 2016


xur 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() &&
----------------
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.


http://reviews.llvm.org/D19515





More information about the llvm-commits mailing list