Diego Novillo
2015-May-12 17:55 UTC
[LLVMdev] Duplicate records in instrumented profile data?
As a follow up to my function entry count
(http://reviews.llvm.org/D9628), I modified codegen to emit the entry
counters:
--- a/lib/CodeGen/CodeGenPGO.cpp
+++ b/lib/CodeGen/CodeGenPGO.cpp
@@ -773,6 +773,8 @@
CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader
*PGOReader,
// Turn on Cold attribute for cold functions.
// FIXME: 1% is from preliminary tuning on SPEC, it may not be optimal.
Fn->addFnAttr(llvm::Attribute::Cold);
+
+ Fn->setEntryCount(FunctionCount);
}
void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S)
{
This is causing a couple of failures in
projects/compiler-rt/test/profile/instrprof-dynamic-*-shared.test.
There is a comparison failure in the bytecode files between the shared
and static link cases.
The entry counts in the statically linked binary are exactly twice
those of the share-linked library. At first I thought it was
llvm-profdata somehow merging the wrong records, but the raw count
file seems to be wrong.
I modified llvm-profdata to show me the records it's merging. And it
is, indeed, getting duplicate records for one of the template
functions. For instrproof-dynamic-one-shared.test, I get the following
for the statically linked profile:
$ bin/llvm-profdata merge -o /dev/null
projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-static.profraw
Added record for: _Z1av - hash: 10 - counts: { 1 1 }
Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 }
Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 }
Added record for: _Z1bv - hash: 10 - counts: { 1 1 }
Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 }
Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 }
Added record for: _Z3fooi - hash: 10 - counts: { 1 1 }
Added record for: main - hash: 0 - counts: { 1 }
Notice how bar<void>() (_Z3barIvEvv) shows up twice. The entry counter
for it ends up at 6 because of this.
For the share-linked profile, I get:
bin/llvm-profdata merge -o /dev/null
projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-shared.profraw
Added record for: _Z1bv - hash: 10 - counts: { 1 1 }
Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 }
Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 }
Added record for: _Z3fooi - hash: 10 - counts: { 1 1 }
Added record for: main - hash: 0 - counts: { 1 }
Added record for: _Z1av - hash: 10 - counts: { 1 1 }
Added record for: _Z3barIvEvv - hash: 10 - counts: { 0 0 }
Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 }
In this case, bar<void>() is also mentioned twice, but the second set
of counters is set to 0, so we managed to get it right.
The problem seems to be somewhere in the instrumentation generated.
Do you folks recognize this? To reproduce, you would need to apply
http://reviews.llvm.org/D9628 (it is still under review) and the
patchlet I included at the top of this message.
Thanks. Diego.
Diego Novillo
2015-May-12 17:57 UTC
[LLVMdev] Duplicate records in instrumented profile data?
+cfe-dev On Tue, May 12, 2015 at 1:55 PM, Diego Novillo <dnovillo at google.com> wrote:> As a follow up to my function entry count > (http://reviews.llvm.org/D9628), I modified codegen to emit the entry > counters: > > --- a/lib/CodeGen/CodeGenPGO.cpp > +++ b/lib/CodeGen/CodeGenPGO.cpp > @@ -773,6 +773,8 @@ > CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader > *PGOReader, > // Turn on Cold attribute for cold functions. > // FIXME: 1% is from preliminary tuning on SPEC, it may not be optimal. > Fn->addFnAttr(llvm::Attribute::Cold); > + > + Fn->setEntryCount(FunctionCount); > } > > void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S) { > > > This is causing a couple of failures in > projects/compiler-rt/test/profile/instrprof-dynamic-*-shared.test. > There is a comparison failure in the bytecode files between the shared > and static link cases. > > The entry counts in the statically linked binary are exactly twice > those of the share-linked library. At first I thought it was > llvm-profdata somehow merging the wrong records, but the raw count > file seems to be wrong. > > I modified llvm-profdata to show me the records it's merging. And it > is, indeed, getting duplicate records for one of the template > functions. For instrproof-dynamic-one-shared.test, I get the following > for the statically linked profile: > > $ bin/llvm-profdata merge -o /dev/null > projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-static.profraw > Added record for: _Z1av - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z1bv - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z3fooi - hash: 10 - counts: { 1 1 } > Added record for: main - hash: 0 - counts: { 1 } > > Notice how bar<void>() (_Z3barIvEvv) shows up twice. The entry counter > for it ends up at 6 because of this. > > For the share-linked profile, I get: > > bin/llvm-profdata merge -o /dev/null > projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-shared.profraw > Added record for: _Z1bv - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z3fooi - hash: 10 - counts: { 1 1 } > Added record for: main - hash: 0 - counts: { 1 } > Added record for: _Z1av - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 0 0 } > Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 } > > In this case, bar<void>() is also mentioned twice, but the second set > of counters is set to 0, so we managed to get it right. > > The problem seems to be somewhere in the instrumentation generated. > Do you folks recognize this? To reproduce, you would need to apply > http://reviews.llvm.org/D9628 (it is still under review) and the > patchlet I included at the top of this message. > > > Thanks. Diego.
Justin Bogner
2015-May-12 18:24 UTC
[LLVMdev] Duplicate records in instrumented profile data?
Diego Novillo <dnovillo at google.com> writes:> As a follow up to my function entry count > (http://reviews.llvm.org/D9628), I modified codegen to emit the entry > counters: > > --- a/lib/CodeGen/CodeGenPGO.cpp > +++ b/lib/CodeGen/CodeGenPGO.cpp > @@ -773,6 +773,8 @@ > CodeGenPGO::applyFunctionAttributes(llvm::IndexedInstrProfReader > *PGOReader, > // Turn on Cold attribute for cold functions. > // FIXME: 1% is from preliminary tuning on SPEC, it may not be optimal. > Fn->addFnAttr(llvm::Attribute::Cold); > + > + Fn->setEntryCount(FunctionCount); > } > > void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, const Stmt *S) { > > > This is causing a couple of failures in > projects/compiler-rt/test/profile/instrprof-dynamic-*-shared.test. > There is a comparison failure in the bytecode files between the shared > and static link cases. > > The entry counts in the statically linked binary are exactly twice > those of the share-linked library. At first I thought it was > llvm-profdata somehow merging the wrong records, but the raw count > file seems to be wrong. > > I modified llvm-profdata to show me the records it's merging. And it > is, indeed, getting duplicate records for one of the template > functions. For instrproof-dynamic-one-shared.test, I get the following > for the statically linked profile: > > $ bin/llvm-profdata merge -o /dev/null > projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-static.profraw > Added record for: _Z1av - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z1bv - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z3fooi - hash: 10 - counts: { 1 1 } > Added record for: main - hash: 0 - counts: { 1 } > > Notice how bar<void>() (_Z3barIvEvv) shows up twice. The entry counter > for it ends up at 6 because of this. > > For the share-linked profile, I get: > > bin/llvm-profdata merge -o /dev/null > projects/compiler-rt/test/profile/Output/instrprof-dynamic-one-shared.test.tmp-shared.profraw > Added record for: _Z1bv - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 3 3 } > Added record for: _Z3barIiEvv - hash: 10 - counts: { 1 1 } > Added record for: _Z3fooi - hash: 10 - counts: { 1 1 } > Added record for: main - hash: 0 - counts: { 1 } > Added record for: _Z1av - hash: 10 - counts: { 1 1 } > Added record for: _Z3barIvEvv - hash: 10 - counts: { 0 0 } > Added record for: _Z3barIcEvv - hash: 10 - counts: { 1 1 } > > In this case, bar<void>() is also mentioned twice, but the second set > of counters is set to 0, so we managed to get it right. > > The problem seems to be somewhere in the instrumentation generated. > Do you folks recognize this? To reproduce, you would need to apply > http://reviews.llvm.org/D9628 (it is still under review) and the > patchlet I included at the top of this message.This is a bug in how we're dealing with the linkage of counters, and it sounds kind of bad to me. Could you file a PR? If you want to investigate I can probably point you in the right direction, otherwise I'll look at this tomorrow.