Uniuqe Names for Internal Linkage Symbols.

This is a standalone patch and this would help Propeller do a better job of code
layout as it can accurately attribute the profiles to the right internal linkage
function.

This also helps SampledFDO/AutoFDO correctly associate sampled profiles to the
right internal function. Currently, if there is more than one internal symbol
foo, their profiles are aggregated by SampledFDO.

This patch adds a new clang option, -funique-internal-funcnames, to generate
unique names for functions with internal linkage. This patch appends the md5
hash of the module name to the function symbol as a best effort to generate a
unique name for symbols with internal linkage.

Differential Revision: https://reviews.llvm.org/D73307

clang/docs/UsersManual.rst

@@ -1677,6 +1677,27 @@ are listed below.
    on ELF targets when using the integrated assembler. This flag currently
    only has an effect on ELF targets.
 
+**-f[no]-unique-internal-linkage-names**
+
+   Controls whether Clang emits a unique (best-effort) symbol name for internal
+   linkage symbols.  When this option is set, compiler hashes the main source
+   file path from the command line and appends it to all internal symbols. If a
+   program contains multiple objects compiled with the same command-line source
+   file path, the symbols are not guaranteed to be unique.  This option is
+   particularly useful in attributing profile information to the correct
+   function when multiple functions with the same private linkage name exist
+   in the binary.
+
+   It should be noted that this option cannot guarantee uniqueness and the
+   following is an example where it is not unique when two modules contain
+   symbols with the same private linkage name:
+
+   .. code-block:: console
+
+     $ cd $P/foo && clang -c -funique-internal-linkage-names name_conflict.c
+     $ cd $P/bar && clang -c -funique-internal-linkage-names name_conflict.c
+     $ cd $P && clang foo/name_conflict.o && bar/name_conflict.o
+
 Profile Guided Optimization
 ---------------------------
 

clang/include/clang/Basic/CodeGenOptions.def

@@ -161,6 +161,7 @@ CODEGENOPT(ReciprocalMath    , 1, 0) ///< Allow FP divisions to be reassociated.
 CODEGENOPT(NoTrappingMath    , 1, 0) ///< Set when -fno-trapping-math is enabled.
 CODEGENOPT(NoNaNsFPMath      , 1, 0) ///< Assume FP arguments, results not NaN.
 CODEGENOPT(CorrectlyRoundedDivSqrt, 1, 0) ///< -cl-fp32-correctly-rounded-divide-sqrt
+CODEGENOPT(UniqueInternalLinkageNames, 1, 0) ///< Internal Linkage symbols get unique names.
 
 /// When false, this attempts to generate code as if the result of an
 /// overflowing conversion matches the overflowing behavior of a target's native

clang/include/clang/Driver/Options.td

@@ -2001,6 +2001,12 @@ def funique_section_names : Flag <["-"], "funique-section-names">,
 def fno_unique_section_names : Flag <["-"], "fno-unique-section-names">,
   Group<f_Group>, Flags<[CC1Option]>;
 
+def funique_internal_linkage_names : Flag <["-"], "funique-internal-linkage-names">,
+  Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Uniqueify Internal Linkage Symbol Names by appending the MD5 hash of the module path">;
+def fno_unique_internal_linkage_names : Flag <["-"], "fno-unique-internal-linkage-names">,
+  Group<f_Group>;
+
 def fstrict_return : Flag<["-"], "fstrict-return">, Group<f_Group>,
   HelpText<"Always treat control flow paths that fall off the end of a "
            "non-void function as unreachable">;

clang/lib/CodeGen/BackendUtil.cpp

@@ -77,6 +77,7 @@
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
+#include "llvm/Transforms/Utils/UniqueInternalLinkageNames.h"
 #include <memory>
 using namespace clang;
 using namespace llvm;
@@ -719,6 +720,12 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
   if (!CodeGenOpts.RewriteMapFiles.empty())
     addSymbolRewriterPass(CodeGenOpts, &MPM);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage symbols
+  // with unique names.
+  if (CodeGenOpts.UniqueInternalLinkageNames) {
+    MPM.add(createUniqueInternalLinkageNamesPass());
+  }
+
   if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts)) {
     MPM.add(createGCOVProfilerPass(*Options));
     if (CodeGenOpts.getDebugInfo() == codegenoptions::NoDebugInfo)
@@ -1203,6 +1210,12 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
       if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
         MPM.addPass(createModuleToFunctionPassAdaptor(BoundsCheckingPass()));
 
+      // Add UniqueInternalLinkageNames Pass which renames internal linkage
+      // symbols with unique names.
+      if (CodeGenOpts.UniqueInternalLinkageNames) {
+        MPM.addPass(UniqueInternalLinkageNamesPass());
+      }
+
       // Lastly, add semantically necessary passes for LTO.
       if (IsLTO || IsThinLTO) {
         MPM.addPass(CanonicalizeAliasesPass());
@@ -1292,6 +1305,12 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
           MPM.addPass(InstrProfiling(*Options, false));
         });
 
+      // Add UniqueInternalLinkageNames Pass which renames internal linkage
+      // symbols with unique names.
+      if (CodeGenOpts.UniqueInternalLinkageNames) {
+        MPM.addPass(UniqueInternalLinkageNamesPass());
+      }
+
       if (IsThinLTO) {
         MPM = PB.buildThinLTOPreLinkDefaultPipeline(
             Level, CodeGenOpts.DebugPassManager);

clang/lib/Driver/ToolChains/Clang.cpp

@@ -4253,6 +4253,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
         options::OPT_fno_function_sections,
         options::OPT_fdata_sections,
         options::OPT_fno_data_sections,
+        options::OPT_funique_internal_linkage_names,
+        options::OPT_fno_unique_internal_linkage_names,
         options::OPT_funique_section_names,
         options::OPT_fno_unique_section_names,
         options::OPT_mrestrict_it,
@@ -4872,6 +4874,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                     options::OPT_fno_unique_section_names, true))
     CmdArgs.push_back("-fno-unique-section-names");
 
+  if (Args.hasFlag(options::OPT_funique_internal_linkage_names,
+                   options::OPT_fno_unique_internal_linkage_names, false))
+    CmdArgs.push_back("-funique-internal-linkage-names");
+
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
                   options::OPT_finstrument_functions_after_inlining,
                   options::OPT_finstrument_function_entry_bare);

clang/lib/Frontend/CompilerInvocation.cpp

@@ -957,6 +957,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.DataSections = Args.hasArg(OPT_fdata_sections);
   Opts.StackSizeSection = Args.hasArg(OPT_fstack_size_section);
   Opts.UniqueSectionNames = !Args.hasArg(OPT_fno_unique_section_names);
+  Opts.UniqueInternalLinkageNames =
+      Args.hasArg(OPT_funique_internal_linkage_names);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 

clang/test/CodeGen/unique-internal-linkage-names.cpp

@@ -0,0 +1,61 @@
+// This test checks if internal linkage symbols get unique names with
+// -funique-internal-linkage-names option.
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+// RUN: %clang_cc1 -triple x86_64 -x c++ -S -emit-llvm -fexperimental-new-pass-manager -funique-internal-linkage-names -o - < %s | FileCheck %s --check-prefix=UNIQUE
+
+static int glob;
+static int foo() {
+  return 0;
+}
+
+int (*bar())() {
+  return foo;
+}
+
+int getGlob() {
+  return glob;
+}
+
+// Function local static variable and anonymous namespace namespace variable.
+namespace {
+int anon_m;
+int getM() {
+  return anon_m;
+}
+} // namespace
+
+int retAnonM() {
+  static int fGlob;
+  return getM() + fGlob;
+}
+
+// Multiversioning symbols
+__attribute__((target("default"))) static int mver() {
+  return 0;
+}
+
+__attribute__((target("sse4.2"))) static int mver() {
+  return 1;
+}
+
+int mver_call() {
+  return mver();
+}
+
+// PLAIN: @_ZL4glob = internal global
+// PLAIN: @_ZZ8retAnonMvE5fGlob = internal global
+// PLAIN: @_ZN12_GLOBAL__N_16anon_mE = internal global
+// PLAIN: define internal i32 @_ZL3foov()
+// PLAIN: define internal i32 @_ZN12_GLOBAL__N_14getMEv
+// PLAIN: define weak_odr i32 ()* @_ZL4mverv.resolver()
+// PLAIN: define internal i32 @_ZL4mverv()
+// PLAIN: define internal i32 @_ZL4mverv.sse4.2()
+// UNIQUE: @_ZL4glob.{{[0-9a-f]+}} = internal global
+// UNIQUE: @_ZZ8retAnonMvE5fGlob.{{[0-9a-f]+}} = internal global
+// UNIQUE: @_ZN12_GLOBAL__N_16anon_mE.{{[0-9a-f]+}} = internal global
+// UNIQUE: define internal i32 @_ZL3foov.{{[0-9a-f]+}}()
+// UNIQUE: define internal i32 @_ZN12_GLOBAL__N_14getMEv.{{[0-9a-f]+}}
+// UNIQUE: define weak_odr i32 ()* @_ZL4mverv.resolver()
+// UNIQUE: define internal i32 @_ZL4mverv.{{[0-9a-f]+}}()
+// UNIQUE: define internal i32 @_ZL4mverv.sse4.2.{{[0-9a-f]+}}

clang/test/Driver/funique-internal-linkage-names.c

@@ -0,0 +1,4 @@
+// RUN: %clang -### -funique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-OPT %s
+// RUN: %clang -### -funique-internal-linkage-names -fno-unique-internal-linkage-names %s -c 2>&1 | FileCheck -check-prefix=CHECK-NOOPT %s
+// CHECK-OPT: "-funique-internal-linkage-names"
+// CHECK-NOOPT-NOT: "-funique-internal-linkage-names"