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

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.

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."

I noticed that the profd metadata is actually constant data when they do
not contain function addresses, so I've made llvm treat them as
constants in this change when appropriate.

Note: This change shouldn't regress value profiling performance, since
(afaik) indirect callees are usually not marked 'always_inline'.

Testing: check-llvm, check-profile


https://reviews.llvm.org/D34134

Files:
  lib/Transforms/Instrumentation/InstrProfiling.cpp
  test/Instrumentation/InstrProfiling/linkage.ll
  test/Transforms/PGOProfile/comdat_internal.ll


Index: test/Transforms/PGOProfile/comdat_internal.ll
===================================================================
--- test/Transforms/PGOProfile/comdat_internal.ll
+++ test/Transforms/PGOProfile/comdat_internal.ll
@@ -14,7 +14,7 @@
 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
 ; CHECK-NOT: __profn__stdin__foo
 ; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
-; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
+; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private constant { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null
 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
 ; CHECK-SAME: , i8* null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
 ; CHECK: @__llvm_prf_nm
Index: test/Instrumentation/InstrProfiling/linkage.ll
===================================================================
--- test/Instrumentation/InstrProfiling/linkage.ll
+++ test/Instrumentation/InstrProfiling/linkage.ll
@@ -28,7 +28,7 @@
 }
 
 ; COMMON: @"__profc_linkage.ll:foo_internal" = internal global
-; COMMON: @"__profd_linkage.ll:foo_internal" = internal global
+; COMMON: @"__profd_linkage.ll:foo_internal" = internal constant
 define internal void @foo_internal() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([23 x i8], [23 x i8]* @"__profn_linkage.ll:foo_internal", i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
@@ -42,9 +42,9 @@
 }
 
 ; LINUX: @__profc_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_cnts", comdat($__profv_foo_extern), align 8
-; LINUX: @__profd_foo_extern = linkonce_odr hidden global {{.*}}section "__llvm_prf_data", comdat($__profv_foo_extern), align 8
+; LINUX: @__profd_foo_extern = linkonce_odr hidden constant {{.*}}section "__llvm_prf_data", comdat($__profv_foo_extern), align 8
 ; OTHER: @__profc_foo_extern = linkonce_odr hidden global
-; OTHER: @__profd_foo_extern = linkonce_odr hidden global
+; OTHER: @__profd_foo_extern = linkonce_odr hidden constant
 define available_externally void @foo_extern() {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_extern, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
Index: lib/Transforms/Instrumentation/InstrProfiling.cpp
===================================================================
--- lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -343,8 +343,9 @@
 
 static inline bool shouldRecordFunctionAddr(Function *F) {
   // Check the linkage
-  if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage() &&
-      !F->hasAvailableExternallyLinkage())
+  if (F->hasAvailableExternallyLinkage())
+    return false;
+  if (!F->hasLinkOnceLinkage() && !F->hasLocalLinkage())
     return true;
   // Prohibit function address recording if the function is both internal and
   // COMDAT. This avoids the profile data variable referencing internal symbols
@@ -470,9 +471,10 @@
 #define INSTR_PROF_DATA(Type, LLVMType, Name, Init) Init,
 #include "llvm/ProfileData/InstrProfData.inc"
   };
-  auto *Data = new GlobalVariable(*M, DataTy, false, NamePtr->getLinkage(),
-                                  ConstantStruct::get(DataTy, DataVals),
-                                  getVarName(Inc, getInstrProfDataVarPrefix()));
+  auto *Data = new GlobalVariable(
+      *M, DataTy, /*isConstant=*/isa<ConstantPointerNull>(FunctionAddr),
+      NamePtr->getLinkage(), ConstantStruct::get(DataTy, DataVals),
+      getVarName(Inc, getInstrProfDataVarPrefix()));
   Data->setVisibility(NamePtr->getVisibility());
   Data->setSection(getInstrProfSectionName(IPSK_data, TT.getObjectFormat()));
   Data->setAlignment(INSTR_PROF_DATA_ALIGNMENT);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D34134.102290.patch
Type: text/x-patch
Size: 4219 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170613/4053eb01/attachment.bin>