[llvm] r291621 - Revert "[PGO] Turn off comdat renaming in IR PGO by default"
Rong Xu via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 15:54:32 PST 2017
Author: xur
Date: Tue Jan 10 17:54:31 2017
New Revision: 291621
URL: http://llvm.org/viewvc/llvm-project?rev=291621&view=rev
Log:
Revert "[PGO] Turn off comdat renaming in IR PGO by default"
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.
Removed:
llvm/trunk/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
llvm/trunk/test/Transforms/PGOProfile/multiple_hash_profile.ll
Modified:
llvm/trunk/include/llvm/ProfileData/InstrProf.h
llvm/trunk/lib/ProfileData/InstrProf.cpp
llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll
llvm/trunk/test/Transforms/PGOProfile/comdat_rename.ll
llvm/trunk/test/Transforms/PGOProfile/indirect_call_profile.ll
Modified: llvm/trunk/include/llvm/ProfileData/InstrProf.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/InstrProf.h?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ProfileData/InstrProf.h (original)
+++ llvm/trunk/include/llvm/ProfileData/InstrProf.h Tue Jan 10 17:54:31 2017
@@ -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"
Modified: llvm/trunk/lib/ProfileData/InstrProf.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/InstrProf.cpp?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/lib/ProfileData/InstrProf.cpp (original)
+++ llvm/trunk/lib/ProfileData/InstrProf.cpp Tue Jan 10 17:54:31 2017
@@ -811,47 +811,4 @@ bool needsComdatForCounter(const Functio
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
Modified: llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp Tue Jan 10 17:54:31 2017
@@ -32,11 +32,6 @@ cl::opt<bool> DoNameCompression("enable-
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(G
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) {
Modified: llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp Tue Jan 10 17:54:31 2017
@@ -119,7 +119,7 @@ static cl::opt<unsigned> MaxNumAnnotatio
// 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
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.
Removed: llvm/trunk/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext?rev=291620&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext (original)
+++ llvm/trunk/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext (removed)
@@ -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
-
Modified: llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/comdat_internal.ll Tue Jan 10 17:54:31 2017
@@ -4,17 +4,17 @@ target datalayout = "e-m:e-i64:64-f80:12
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
Modified: llvm/trunk/test/Transforms/PGOProfile/comdat_rename.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/comdat_rename.ll?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/comdat_rename.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/comdat_rename.ll Tue Jan 10 17:54:31 2017
@@ -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
+ at 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() {
Modified: llvm/trunk/test/Transforms/PGOProfile/indirect_call_profile.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/indirect_call_profile.ll?rev=291621&r1=291620&r2=291621&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/indirect_call_profile.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/indirect_call_profile.ll Tue Jan 10 17:54:31 2017
@@ -54,7 +54,7 @@ bb11:
}
; 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
Removed: llvm/trunk/test/Transforms/PGOProfile/multiple_hash_profile.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/PGOProfile/multiple_hash_profile.ll?rev=291620&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/PGOProfile/multiple_hash_profile.ll (original)
+++ llvm/trunk/test/Transforms/PGOProfile/multiple_hash_profile.ll (removed)
@@ -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
-
- at 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
-}
-
-
More information about the llvm-commits
mailing list