Skip to content

Commit

Permalink
[PGO] Directory name stripping in global identifier for static functions
Browse files Browse the repository at this point in the history
Current internal option -static-func-full-module-prefix keeps all the
directory path the profile counter names for static functions. The default
of this option is false. This strips the directory names from the source
filename which is problematic:

(1) it creates linker errors for profile-generation compilation, exposed in
our internal benchmarks. We are seeing messages like
"warning: relocation refers to discarded section".
This is due to the name conflicts after the stripping.

(2) the stripping only applies to getPGOFuncName.
Current Thin-LTO module importing for the indirect-calls assumes
the source directory name not being stripped. Current default value
for this option can potentially prevent some inter-module
indirect-call-promotions.

This patch turns the default value for -static-func-full-module-prefix to true.

The second part of the patch is to have an alternative implementation under
 the internal option -static-func-strip-dirname-prefix=<value>

This options specifies level of directories to be stripped from the source
filename. Using a large value as the parameter has the same effect as
-static-func-full-module-prefix.

Differential Revision: http://reviews.llvm.org/D29512

llvm-svn: 296206
  • Loading branch information
xur-llvm committed Feb 25, 2017
1 parent 3fc6fb7 commit a1a9f70
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
35 changes: 34 additions & 1 deletion llvm/lib/ProfileData/InstrProf.cpp
Expand Up @@ -29,10 +29,23 @@
using namespace llvm;

static cl::opt<bool> StaticFuncFullModulePrefix(
"static-func-full-module-prefix", cl::init(false),
"static-func-full-module-prefix", cl::init(true),
cl::desc("Use full module build paths in the profile counter names for "
"static functions."));

// This option is tailored to users that have different top-level directory in
// profile-gen and profile-use compilation. Users need to specific the number
// of levels to strip. A value larger than the number of directories in the
// source file will strip all the directory names and only leave the basename.
//
// Note current Thin-LTO module importing for the indirect-calls assumes
// the source directory name not being stripped. A non-zero option value here
// can potentially prevent some inter-module indirect-call-promotions.
static cl::opt<unsigned> StaticFuncStripDirNamePrefix(
"static-func-strip-dirname-prefix", cl::init(0),
cl::desc("Strip specified level of directory name from source path in "
"the profile counter name for static functions."));

namespace {
std::string getInstrProfErrString(instrprof_error Err) {
switch (Err) {
Expand Down Expand Up @@ -133,6 +146,24 @@ std::string getPGOFuncName(StringRef RawFuncName,
return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
}

// Strip NumPrefix level of directory name from PathNameStr. If the number of
// directory separators is less than NumPrefix, strip all the directories and
// leave base file name only.
static StringRef stripDirPrefix(StringRef PathNameStr, uint32_t NumPrefix) {
uint32_t Count = NumPrefix;
uint32_t Pos = 0, LastPos = 0;
for (auto & CI : PathNameStr) {
++Pos;
if (llvm::sys::path::is_separator(CI)) {
LastPos = Pos;
--Count;
}
if (Count == 0)
break;
}
return PathNameStr.substr(LastPos);
}

// Return the PGOFuncName. This function has some special handling when called
// in LTO optimization. The following only applies when calling in LTO passes
// (when \c InLTO is true): LTO's internalization privatizes many global linkage
Expand All @@ -151,6 +182,8 @@ std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) {
StringRef FileName = (StaticFuncFullModulePrefix
? F.getParent()->getName()
: sys::path::filename(F.getParent()->getName()));
if (StaticFuncFullModulePrefix && StaticFuncStripDirNamePrefix != 0)
FileName = stripDirPrefix(FileName, StaticFuncStripDirNamePrefix);
return getPGOFuncName(F.getName(), F.getLinkage(), FileName, Version);
}

Expand Down
11 changes: 8 additions & 3 deletions llvm/test/Transforms/PGOProfile/statics_counter_naming.ll
@@ -1,9 +1,14 @@
; RUN: opt %s -pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
; RUN: opt %s -passes=pgo-instr-gen -S | FileCheck %s --check-prefix=GEN
; RUN: opt %s -pgo-instr-gen -static-func-full-module-prefix=false -S | FileCheck %s --check-prefix=NOPATH
; RUN: opt %s -passes=pgo-instr-gen -static-func-full-module-prefix=false -S | FileCheck %s --check-prefix=NOPATH
; RUN: opt %s --pgo-instr-gen -static-func-strip-dirname-prefix=1000 -S | FileCheck %s --check-prefix=NOPATH
; RUN: opt %s -passes=pgo-instr-gen -static-func-strip-dirname-prefix=1000 -S | FileCheck %s --check-prefix=NOPATH
; RUN: opt %s --pgo-instr-gen -static-func-strip-dirname-prefix=1 -S | FileCheck %s --check-prefix=HASPATH
; RUN: opt %s -passes=pgo-instr-gen -static-func-strip-dirname-prefix=1 -S | FileCheck %s --check-prefix=HASPATH
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

; GEN: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll:func"
; NOPATH: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll:func"
; HASPATH-NOT: @__profn_statics_counter_naming.ll_func = private constant [30 x i8] c"statics_counter_naming.ll:func"

define internal i32 @func() {
entry:
Expand Down

0 comments on commit a1a9f70

Please sign in to comment.