PGO] Fix __llvm_profile_raw_version linkage in MACHO

[xur]

Summary

IR instrumentation generates a COMDAT symbol __llvm_profile_raw_version to overwrite the same symbol in profile run-time to distinguish IR profiles from Clang generated profiles. In MACHO, LinkOnceODR linkage is used due to the lack of COMDAT support.

But LinkOnceODR linkage might have .weak_def_can_be_hidden assembly directive, while the weak variable in run-time has a .weak_definition directive. Linker will not merge these two symbols even they have the same name. The end result is IR profiles are not properly flagged in MACHO.

This patch changes the linkage for __llvm_profile_raw_version in each module to LinkOnceAny so that it has same .weak_definition directive as in the run-time..

[vsk]

Wow, so without this fix are IR profiles unusable on macho? I'm not sure if I'm understanding this correctly because it doesn't seem like that could pass testing.

Also, is "weak_def_can_be_hidden" incorrect because __llvm_profile_raw_version isn't supposed to be "hidden"? I'd appreciate it if you could point me to any resources that discuss the difference between weak_def_can_be_hidden and weak_definition -- if there aren't any I can dig a bit more :).

[xur]

In D20078#425034, @vsk wrote:

Wow, so without this fix are IR profiles unusable on macho? I'm not sure if I'm understanding this correctly because it doesn't seem like that could pass testing.

Unfortunately, this is true: IR profiles is broken in MACHO for profile-use. We don't have an end-to-end PGO test for IR profiles. I will add later.

Also, is "weak_def_can_be_hidden" incorrect because __llvm_profile_raw_version isn't supposed to be "hidden"? I'd appreciate it if you could point me to any resources that discuss the difference between weak_def_can_be_hidden and weak_definition -- if there aren't any I can dig a bit more :).

Our tests show that what attribute (weak_def_can_be_hidden or weak_definition) to use does not seem to matter --- we just cannot mix this two. I chose weak_definition because it's easier to write the code in the profile run-time (just use attribute((weak)) as it's). It's harder to write code to generate the weak_def_can_be_hidden attribute.

We thought apple has better documentation about weak_def_can_be_hidden because this seems to be macho specific. I only find a few references online. Here is one:
 http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20131028/193223.html

[vsk]

Our tests show that what attribute (weak_def_can_be_hidden or weak_definition) to use does not seem to matter --- we just cannot mix this two. I chose weak_definition because it's easier to write the code in the profile run-time (just use attribute((weak)) as it's). It's harder to write code to generate the weak_def_can_be_hidden attribute.

Got it. According to that commit, weak_def_can_be_hidden is appropriate if the symbol's address isn't taken. It seems like .weak_definition is more suitable here, then.

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

static void createIRLevelProfileFlagVariable(Module &M) {
   uint64_t ProfileVersion = (INSTR_PROF_RAW_VERSION | VARIANT_MASK_IR_PROF);
   auto IRLevelVersionVariable = new GlobalVariable(
       M, IntTy64, true, GlobalVariable::ExternalLinkage,
       Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)),
       INSTR_PROF_QUOTE(IR_LEVEL_PROF_VERSION_VAR));
   IRLevelVersionVariable->setVisibility(GlobalValue::DefaultVisibility);
   Triple TT(M.getTargetTriple());
   if (TT.isOSBinFormatMachO())
-    IRLevelVersionVariable->setLinkage(GlobalValue::LinkOnceODRLinkage);
+    IRLevelVersionVariable->setLinkage(GlobalValue::LinkOnceAnyLinkage);
   else
     IRLevelVersionVariable->setComdat(M.getOrInsertComdat(
         StringRef(INSTR_PROF_QUOTE(IR_LEVEL_PROF_VERSION_VAR))));
 }
 
 static bool InstrumentAllFunctions(
     Module &M, function_ref<BranchProbabilityInfo &(Function &)> LookupBPI,
     function_ref<BlockFrequencyInfo &(Function &)> LookupBFI) {

llvm/trunk/test/Transforms/PGOProfile/macho.ll

+; RUN: opt < %s -mtriple=x86_64-apple-macosx10.11.0 -pgo-instr-gen -instrprof -S | llc | FileCheck %s --check-prefix=MACHO-DIRECTIVE
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+
+; MACHO-DIRECTIVE: .weak_definition        ___llvm_profile_raw_version
+define i32 @test_macho(i32 %i) {
+entry:
+  ret i32 %i
+}