[PATCH] D12781: PGO IR-level instrumentation infrastructure

[Sean Silva via llvm-commits]

On Tue, Nov 24, 2015 at 4:07 PM, Rong Xu <xur at google.com> wrote:

> Hi Sean,
>
> Sorry about this. I revert the change a moment ago.
>
> As for the test. Here is the way I want to fix:
> In profile-generate tests, I will test (1) number of counters inserted in
> each function, and (2) If there is a instrumentation in a specific bb. I
> will skip the count index.
>
> In profile-use, I will invoke a new pass branch-prob that computes the
> branch probability.  I will check the branch probability rather the branch
> weight metadata.
>
> Let me know if you think this is reasonable, or you have a better way to
> do the test.
>

You can use FileCheck variable captures (
http://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-variables) to
match up the indexes (see the example where it captures a register
assignment and checks that a later instruction uses that register).
Together with good use of CHECK lines, you should be able to capture
exactly what you need and make the test very readable.

The profile-use pass only adds metadata, so we should be checking the
metadata. We should not need to invoke branch-prob to test the behavior.

Also, Hal's point is worth investigating: why is this pass not
deterministic across platforms? LLVM code should have deterministic output
independent of the host platform.

-- Sean Silva


>
> Thanks,
>
> -Rong
>
>
>
> On Tue, Nov 24, 2015 at 3:16 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>> Hi Rong,
>>
>> Typically, when there have been multiple active reviewers, it is common
>> courtesy to wait for all of them to LGTM the patch.
>>
>> I had further comments on the testing here (which the bots seem to have
>> caught you on anyway), so I recommend reverting this patch for the moment
>> and reopening the review.
>>
>> -- Sean Silva
>>
>>
>> On Tue, Nov 24, 2015 at 1:34 PM, Rong Xu <xur at google.com> wrote:
>>
>>> This revision was automatically updated to reflect the committed changes.
>>> Closed by commit rL254021: [PGO] MST based PGO instrumentation
>>> infrastructure (authored by xur).
>>>
>>> Changed prior to commit:
>>>   http://reviews.llvm.org/D12781?vs=41062&id=41079#toc
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> http://reviews.llvm.org/D12781
>>>
>>> Files:
>>>   llvm/trunk/include/llvm/IR/DiagnosticInfo.h
>>>   llvm/trunk/include/llvm/InitializePasses.h
>>>   llvm/trunk/include/llvm/LinkAllPasses.h
>>>   llvm/trunk/include/llvm/Transforms/Instrumentation.h
>>>   llvm/trunk/lib/IR/DiagnosticInfo.cpp
>>>   llvm/trunk/lib/Transforms/IPO/LLVMBuild.txt
>>>   llvm/trunk/lib/Transforms/Instrumentation/CFGMST.h
>>>   llvm/trunk/lib/Transforms/Instrumentation/CMakeLists.txt
>>>   llvm/trunk/lib/Transforms/Instrumentation/Instrumentation.cpp
>>>   llvm/trunk/lib/Transforms/Instrumentation/LLVMBuild.txt
>>>   llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/branch1.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/branch2.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/criticaledge.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/landingpad.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/loop1.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/loop2.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/loop3.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/single_bb.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/Inputs/switch.proftext
>>>   llvm/trunk/test/Transforms/PGOProfile/branch1_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/branch1_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/branch2_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/branch2_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/checksum_mismatch.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/criticaledge_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/criticaledge_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/landingpad_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/landingpad_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop1_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop1_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop2_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop2_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop3_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/loop3_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/noprofile_use.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/single_bb_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/switch_gen.ll
>>>   llvm/trunk/test/Transforms/PGOProfile/switch_use.ll
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151124/1c9a6fb9/attachment.html>