[InstrProf] Don't take the address of alwaysinline available_externally functions

[vsk]

Summary

Doing so breaks compilation of the following C program
(under -fprofile-instr-generate):

__attribute__((always_inline)) inline int foo() { return 0; }

int main() { return foo(); }

At link time, we fail because taking the address of an
available_externally function creates an undefined external reference,
which the TU cannot provide.

Emitting the function definition into the object file at all appears to
be a violation of the langref: "Globals with 'available_externally'
linkage are never emitted into the object file corresponding to the LLVM
module."

Testing: check-llvm, check-profile

[davidxl]

lib/Transforms/Instrumentation/InstrProfiling.cpp

346 ↗

(On Diff #102290)

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.

475 ↗

(On Diff #102290)

This can not be readonly -- there is a pointer to the value profiling counter array from here.

[vsk]

• Only change behavior when handling alwaysinline + available_externally functions.

[davidxl]

lgtm ( The area of linkage handling has many subtle problems in the past with large C++ apps, so be prepared for any breakage introduced).

llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp

static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) {
   SmallVector<char, 24> HashPostfix;
   if (Name.endswith((Twine(".") + Twine(FuncHash)).toStringRef(HashPostfix)))
     return (Prefix + Name).str();
   return (Prefix + Name + "." + Twine(FuncHash)).str();
 }
 
 static inline bool shouldRecordFunctionAddr(Function *F) {
   // Check the linkage
+  bool HasAvailableExternallyLinkage = F->hasAvailableExternallyLinkage();
   if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&
-      !F->hasAvailableExternallyLinkage())
+      !HasAvailableExternallyLinkage)
     return true;
+
+  // A function marked 'alwaysinline' with available_externally linkage can't
+  // have its address taken. Doing so would create an undefined external ref to
+  // the function, which would fail to link.
+  if (HasAvailableExternallyLinkage &&
+      F->hasFnAttribute(Attribute::AlwaysInline))
+    return false;
+
   // Prohibit function address recording if the function is both internal and
   // COMDAT. This avoids the profile data variable referencing internal symbols
   // in COMDAT.
   if (F->hasLocalLinkage() && F->hasComdat())
     return false;
+
   // Check uses of this function for other than direct calls or invokes to it.
   // Inline virtual functions have linkeOnceODR linkage. When a key method
   // exists, the vtable will only be emitted in the TU where the key method
   // is defined. In a TU where vtable is not available, the function won't
   // be 'addresstaken'. If its address is not recorded here, the profile data
   // with missing address may be picked by the linker leading  to missing
   // indirect call target info.
   return F->hasAddressTaken() || F->hasLinkOnceLinkage();

llvm/trunk/test/Instrumentation/InstrProfiling/always_inline.ll

+; Check that '__attribute__((always_inline)) inline' functions are inlined.
+
+; RUN: opt < %s -mtriple=x86_64-apple-macosx10.10.0 -instrprof -inline -S | FileCheck %s 
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.13.0"
+
+@__profn_foo = linkonce_odr hidden constant [3 x i8] c"foo"
+
+; CHECK-LABEL: @main
+; CHECK-NOT: call
+define i32 @main() {
+entry:
+  %call = call i32 @foo()
+  ret i32 %call
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #0
+
+; CHECK-NOT: define available_externally i32 @foo
+define available_externally i32 @foo() #1 {
+entry:
+  call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @__profn_foo, i32 0, i32 0), i64 0, i32 1, i32 0)
+  ret i32 0
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { alwaysinline }