On Fri, Dec 11, 2015 at 6:19 PM, Justin Bogner <m...@justinbogner.com> wrote:
> Easwaran Raman <era...@google.com> writes:
>> eraman updated this revision to Diff 42549.
>> eraman added a comment.
>>
>> Added a test case.
>>
>>
>> Repository:
>> rL LLVM
>>
>> http://reviews.llvm.org/D15163
>>
>> Files:
>> lib/CodeGen/CodeGenModule.cpp
>> test/CodeGen/pgo-max-function-count.c
>>
>> Index: test/CodeGen/pgo-max-function-count.c
>> ===================================================================
>> --- /dev/null
>> +++ test/CodeGen/pgo-max-function-count.c
>> @@ -0,0 +1,9 @@
>> +// RUN: %clang -fprofile-generate -o %t -O2 %s
>> +// RUN: env LLVM_PROFILE_FILE=%t.profraw %t
>
> The clang tests are run in environments where the generated binaries
> can't run on the host system, so you can't do this. Instead of this kind
> of full integration test you should provide handwritten profile data to
> feed into the test. See the tests in test/Profile for examples, and also
> consider putting this test in that directory with the others.
>
This is a good suggestion -- I previously suggested adding a test in
compiler-rt and get rid of this one, but looks like we can do what
Justin suggested. You can generate profile data in text format like
this:
llvm-profdata merge -o pgo-max-function-count.proftxt -text
<raw_profile_data_generated_on_host>
After adding pgo-max-function-count.proftxt into
tools/clang/test/Profile/Inputs, the test can be modified to do
profile-use compile only.
David
>> +// RUN: llvm-profdata merge -o %t.profdata %t.profraw
>> +// RUN: %clang -fprofile-use=%t.profdata -o - -S -emit-llvm %s | FileCheck
>> %s
>> +// Check
>> +int main() {
>> + return 0;
>> +}
>> +// CHECK: !{{[0-9]+}} = !{i32 1, !"MaxFunctionCount", i32 1}
>> Index: lib/CodeGen/CodeGenModule.cpp
>> ===================================================================
>> --- lib/CodeGen/CodeGenModule.cpp
>> +++ lib/CodeGen/CodeGenModule.cpp
>> @@ -376,6 +376,9 @@
>> }
>> if (PGOReader && PGOStats.hasDiagnostics())
>> PGOStats.reportDiagnostics(getDiags(), getCodeGenOpts().MainFileName);
>> + // In PGO mode, attach maximum function count to the module.
>
> This comment isn't helpful - it's just stating exactly what the code
> that follows does.
>
>> + if (PGOReader)
>> +
>> getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount());
>
> It would read better to fold the two `if (PGOReader)` checks
> together. ie:
>
> if (PGOReader) {
> getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount());
> if (PGOStats.hasDiagnostics())
> PGOStats.reportDiagnostics(getDiags(), getCodeGenOpts().MainFileName);
> }
>
> That said, wouldn't it make more sense to set this within PGOReader
> itself? It feels pretty awkward for this to be happening in
> CodeGenModule::Release().
>
>> EmitCtorList(GlobalCtors, "llvm.global_ctors");
>> EmitCtorList(GlobalDtors, "llvm.global_dtors");
>> EmitGlobalAnnotations();
>>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
