RPCSX/llvm
1
0
Fork 0
mirror of https://github.com/RPCSX/llvm.git synced 2024-11-23 19:59:57 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Revert "[PGO] Turn off comdat renaming in IR PGO by default"

Browse Source 
This patch reverts r291588: [PGO] Turn off comdat renaming in IR PGO by default,
as we are seeing some hash mismatches in our internal tests.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@291621 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Rong Xu 2017-01-10 23:54:31 +00:00
parent e9fdb8bc46
commit de4116d55c
9 changed files with 36 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
include/llvm/ProfileData/InstrProf.h
View File

@ -230,15 +230,6 @@ class InstrProfSymtab;
/// bytes. This method decodes the string and populates the \c Symtab.
Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab);
/// Check if INSTR_PROF_RAW_VERSION_VAR is defined. This global is only being
/// set in IR PGO compilation.
bool isIRPGOFlagSet(const Module *M);
/// Check if we can safely rename this Comdat function. Instances of the same
/// comdat function may have different control flows thus can not share the
/// same counter variable.
bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken = false);
enum InstrProfValueKind : uint32_t {
#define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
#include "llvm/ProfileData/InstrProfData.inc"

43
lib/ProfileData/InstrProf.cpp
View File

@ -811,47 +811,4 @@ bool needsComdatForCounter(const Function &F, const Module &M) {
return true;
}
// Check if INSTR_PROF_RAW_VERSION_VAR is defined.
bool isIRPGOFlagSet(const Module *M) {
auto IRInstrVar =
M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
if (!IRInstrVar || IRInstrVar->isDeclaration() ||
IRInstrVar->hasLocalLinkage())
return false;
// Check if the flag is set.
if (!IRInstrVar->hasInitializer())
return false;
const Constant *InitVal = IRInstrVar->getInitializer();
if (!InitVal)
return false;
return (dyn_cast<ConstantInt>(InitVal)->getZExtValue() &
VARIANT_MASK_IR_PROF) != 0;
}
// Check if we can safely rename this Comdat function.
bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
if (F.getName().empty())
return false;
if (!needsComdatForCounter(F, *(F.getParent())))
return false;
// Unsafe to rename the address-taken function (which can be used in
// function comparison).
if (CheckAddressTaken && F.hasAddressTaken())
return false;
// Only safe to do if this function may be discarded if it is not used
// in the compilation unit.
if (!GlobalValue::isDiscardableIfUnused(F.getLinkage()))
return false;
// For AvailableExternallyLinkage functions.
if (!F.hasComdat()) {
assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
return true;
}
return true;
}
} // end namespace llvm

16
lib/Transforms/Instrumentation/InstrProfiling.cpp
View File

@ -32,11 +32,6 @@ cl::opt<bool> DoNameCompression("enable-name-compression",
cl::desc("Enable name string compression"),
cl::init(true));
cl::opt<bool> DoHashBasedCounterSplit(
"hash-based-counter-split",
cl::desc("Rename counter variable of a comdat function based on cfg hash"),
cl::init(true));
cl::opt<bool> ValueProfileStaticAlloc(
"vp-static-alloc",
cl::desc("Do static counter allocation for value profiler"),
@ -277,16 +272,7 @@ void InstrProfiling::lowerCoverageData(GlobalVariable *CoverageNamesVar) {
static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) {
StringRef NamePrefix = getInstrProfNameVarPrefix();
StringRef Name = Inc->getName()->getName().substr(NamePrefix.size());
Function *F = Inc->getParent()->getParent();
Module *M = F->getParent();
if (!DoHashBasedCounterSplit || !isIRPGOFlagSet(M) ||
!canRenameComdatFunc(*F))
return (Prefix + Name).str();
uint64_t FuncHash = Inc->getHash()->getZExtValue();
SmallVector<char, 24> HashPostfix;
if (Name.endswith((Twine(".") + Twine(FuncHash)).toStringRef(HashPostfix)))
return (Prefix + Name).str();
return (Prefix + Name + "." + Twine(FuncHash)).str();
return (Prefix + Name).str();
}
static inline bool shouldRecordFunctionAddr(Function *F) {

16
lib/Transforms/Instrumentation/PGOInstrumentation.cpp
View File

@ -119,7 +119,7 @@ static cl::opt<unsigned> MaxNumAnnotations(
// Command line option to control appending FunctionHash to the name of a COMDAT
// function. This is to avoid the hash mismatch caused by the preinliner.
static cl::opt<bool> DoComdatRenaming(
"do-comdat-renaming", cl::init(false), cl::Hidden,
"do-comdat-renaming", cl::init(true), cl::Hidden,
cl::desc("Append function hash to the name of COMDAT function to avoid "
"function hash mismatch due to the preinliner"));
@ -407,8 +407,20 @@ void FuncPGOInstrumentation<Edge, BBInfo>::computeCFGHash() {
static bool canRenameComdat(
Function &F,
std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
if (!DoComdatRenaming || !canRenameComdatFunc(F, true))
if (F.getName().empty())
return false;
if (!needsComdatForCounter(F, *(F.getParent())))
return false;
// Only safe to do if this function may be discarded if it is not used
// in the compilation unit.
if (!GlobalValue::isDiscardableIfUnused(F.getLinkage()))
return false;
// For AvailableExternallyLinkage functions.
if (!F.hasComdat()) {
assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
return true;
}
// FIXME: Current only handle those Comdat groups that only containing one
// function and function aliases.

36
test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
View File

@ -1,36 +0,0 @@
# IR level Instrumentation Flag
:ir
_Z3fooi
# Func Hash:
72057606922829823
# Num Counters:
2
# Counter Values:
18
12
_Z3fooi
# Func Hash:
12884901887
# Num Counters:
1
# Counter Values:
0
_Z3bari
# Func Hash:
72057606922829823
# Num Counters:
2
# Counter Values:
0
0
_Z4m2f1v
# Func Hash:
12884901887
# Num Counters:
1
# Counter Values:
1

8
test/Transforms/PGOProfile/comdat_internal.ll
View File

@ -4,17 +4,17 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
$foo = comdat any
; CHECK: $foo = comdat any
; CHECK: $foo.[[FOO_HASH:[0-9]+]] = comdat any
; CHECK: $__llvm_profile_raw_version = comdat any
; CHECK: $__profv__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
; CHECK: $__profv__stdin__foo.[[FOO_HASH]] = comdat any
@bar = global i32 ()* @foo, align 8
; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
; CHECK: @__profn__stdin__foo = private constant [11 x i8] c"<stdin>:foo"
; CHECK: @__profn__stdin__foo.[[FOO_HASH]] = private constant [23 x i8] c"<stdin>:foo.[[FOO_HASH]]"
; 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, [1 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 global { i64, i64, i64*, i8*, i8*, i32, [1 x i16] } { i64 6965568665848889497, 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, [1 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
; CHECK: @__llvm_prf_nm

20
test/Transforms/PGOProfile/comdat_rename.ll
View File

@ -1,7 +1,7 @@
; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s
; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s
; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s
; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s
; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s
; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s
; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s
; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s
; Rename Comdat group and its function.
$f = comdat any
@ -38,10 +38,22 @@ define linkonce void @tf2() comdat($tf) {
ret void
}
; Renaming Comdat with aliases.
$f_with_alias = comdat any
; COMMON: $f_with_alias.[[SINGLEBB_HASH]] = comdat any
@af = alias void (...), bitcast (void ()* @f_with_alias to void (...)*)
; COFFONLY: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to
; ELFONLY-DAG: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to
define linkonce_odr void @f_with_alias() comdat($f_with_alias) {
ret void
}
; Rename AvailableExternallyLinkage functions
; ELFONLY-DAG: $aef.[[SINGLEBB_HASH]] = comdat any
; ELFONLY: @f = weak alias void (), void ()* @f.[[SINGLEBB_HASH]]
; ELFONLY: @f_with_alias = weak alias void (), void ()* @f_with_alias.[[SINGLEBB_HASH]]
; ELFONLY: @af = weak alias void (...), void (...)* @af.[[SINGLEBB_HASH]]
; ELFONLY: @aef = weak alias void (), void ()* @aef.[[SINGLEBB_HASH]]
define available_externally void @aef() {

2
test/Transforms/PGOProfile/indirect_call_profile.ll
View File

@ -54,7 +54,7 @@ bb11: ; preds = %bb2
}
; Test that comdat function's address is recorded.
; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@__profc_foo3.[[FOO3_HASH]]
; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@foo3.[[FOO3_HASH]]
; Function Attrs: nounwind uwtable
define linkonce_odr i32 @foo3() comdat {
ret i32 1

36
test/Transforms/PGOProfile/multiple_hash_profile.ll
View File

@ -1,36 +0,0 @@
; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
$_Z3fooi = comdat any
@g2 = local_unnamed_addr global i32 (i32)* null, align 8
define i32 @_Z3bari(i32 %i) {
entry:
%cmp = icmp sgt i32 %i, 2
%mul = select i1 %cmp, i32 1, i32 %i
%retval.0 = mul nsw i32 %mul, %i
ret i32 %retval.0
}
define void @_Z4m2f1v() {
entry:
store i32 (i32)* @_Z3fooi, i32 (i32)** @g2, align 8
ret void
}
define linkonce_odr i32 @_Z3fooi(i32 %i) comdat {
entry:
%cmp.i = icmp sgt i32 %i, 2
%mul.i = select i1 %cmp.i, i32 1, i32 %i
; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
; CHECK-SAME !prof ![[BW:[0-9]+]]
; CHECK ![[BW]] = !{!"branch_weights", i32 12, i32 6}
%retval.0.i = mul nsw i32 %mul.i, %i
ret i32 %retval.0.i
}