[PATCH] D34134: [InstrProf] Don't take the address of available_externally functions

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:346
   // Check the linkage
-  if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&
-      !F->hasAvailableExternallyLinkage())
+  if (F->hasAvailableExternallyLinkage())
+    return false;
----------------
Should you check always inline attribute here too to avoid missing indirect call profiling? AvailableExternally is very common in C++ (e.g, for extern template). Blindly dropping the address can lead to lots of missing value profiling. 


================
Comment at: lib/Transforms/Instrumentation/InstrProfiling.cpp:475
+  auto *Data = new GlobalVariable(
+      *M, DataTy, /*isConstant=*/isa<ConstantPointerNull>(FunctionAddr),
+      NamePtr->getLinkage(), ConstantStruct::get(DataTy, DataVals),
----------------
This can not be readonly  -- there is a pointer to the value profiling counter array from here.


https://reviews.llvm.org/D34134