[PATCH] Assign a low branch weight to ASan's slow path.

[Kostya Serebryany]

I've made more runs over the weekend. Here is how the typical run looks:
       400.perlbench,      1666.00,      1534.00,         0.92
           401.bzip2,       984.00,       890.00,         0.90
             403.gcc,       724.00,       712.00,         0.98
             429.mcf,       411.00,       370.00,         0.90
           445.gobmk,      1095.00,      1011.00,         0.92
           456.hmmer,      1215.00,      1050.00,         0.86
           458.sjeng,      1246.00,      1071.00,         0.86
      462.libquantum,       393.00,       398.00,         1.01
         464.h264ref,      1598.00,      1357.00,         0.85
         471.omnetpp,       702.00,       674.00,         0.96
           473.astar,       675.00,       638.00,         0.95
       483.xalancbmk,       546.00,       519.00,         0.95
            433.milc,       721.00,       680.00,         0.94
            444.namd,       656.00,       641.00,         0.98
          447.dealII,       711.00,       688.00,         0.97
          450.soplex,       364.00,       344.00,         0.95
          453.povray,       531.00,       493.00,         0.93
             470.lbm,       444.00,       435.00,         0.98
         482.sphinx3,      1034.00,       905.00,         0.88

Committed as r216972.
Thanks for reminding us about this low hanging fruit!

--kcc


On Fri, Aug 29, 2014 at 10:28 AM, Kostya Serebryany <kcc at google.com> wrote:

> Here is my data from the last night's run of SPEC ref (the smaller the
> better):
>
>                           w/o weights,  with weights,     diff
>        400.perlbench,      1449.00,      1440.00,         0.99
>            401.bzip2,       905.00,       834.00,         0.92
>              403.gcc,       743.00,       742.00,         1.00
>              429.mcf,       315.00,       338.00,         1.07
>            445.gobmk,       973.00,       882.00,         0.91
>            456.hmmer,      1067.00,      1007.00,         0.94
>            458.sjeng,      1004.00,      1044.00,         1.04
>       462.libquantum,       344.00,       398.00,         1.16
>          464.h264ref,      1485.00,      1277.00,         0.86
>          471.omnetpp,       580.00,       609.00,         1.05
>            473.astar,       575.00,       646.00,         1.12
>        483.xalancbmk,       538.00,       510.00,         0.95
>             433.milc,       650.00,       585.00,         0.90
>             444.namd,       592.00,       601.00,         1.02
>           447.dealII,       625.00,       640.00,         1.02
>           450.soplex,       333.00,       348.00,         1.05
>           453.povray,       493.00,       481.00,         0.98
>              470.lbm,       370.00,       380.00,         1.03
>          482.sphinx3,       868.00,       850.00,         0.98
>
> It's kind of mixed....
> I've just migrated to a new machine so I don't know if the results are
> stable here.
> Let me ponder on this change for a few days (after the weekend)...
>
> --kcc
>
>
>
> On Fri, Aug 29, 2014 at 9:06 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>> I am actually surprised too :)
>> We've used branch weights in a couple of other places but not here.
>> One can easily create a benchmark where these weights will lead to a
>> performance loss,
>> but the average SPEC numbers clearly show the overal benefit.
>> Let me submit this patch once I confirm the SPEC/ref numbers (the
>> SPEC/test numbers I saw yesterday are very nice)
>>
>> --kcc
>>
>>
>> On Fri, Aug 29, 2014 at 1:57 AM, Chandler Carruth <chandlerc at google.com>
>> wrote:
>>
>>> Adding ASan folks...
>>>
>>> Didn't we already look at this Kostya? A bit surprised this wasn't
>>> already the case.
>>>
>>>
>>> On Fri, Aug 29, 2014 at 1:24 AM, Jonas Wagner <jonas.wagner at epfl.ch>
>>> wrote:
>>>
>>>> Experiments on the SPEC benchmarks show that the slow path is rarely
>>>> taken, and that branch weights reduce branch misses and overall runtime.
>>>> ---
>>>>  lib/Transforms/Instrumentation/AddressSanitizer.cpp | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> b/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> index 357b5d5..3ff0a61 100644
>>>> --- a/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> +++ b/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>>> @@ -870,8 +870,11 @@ void
>>>> AddressSanitizer::instrumentAddress(Instruction *OrigIns,
>>>>    TerminatorInst *CrashTerm = nullptr;
>>>>
>>>>    if (ClAlwaysSlowPath || (TypeSize < 8 * Granularity)) {
>>>> +    // We use branch weights for the slow path check, to indicate that
>>>> the slow
>>>> +    // path is rarely taken. This seems to be the case for SPEC
>>>> benchmarks.
>>>>      TerminatorInst *CheckTerm =
>>>> -        SplitBlockAndInsertIfThen(Cmp, InsertBefore, false);
>>>> +        SplitBlockAndInsertIfThen(Cmp, InsertBefore, false,
>>>> +            MDBuilder(*C).createBranchWeights(1, 100000));
>>>>      assert(dyn_cast<BranchInst>(CheckTerm)->isUnconditional());
>>>>      BasicBlock *NextBB = CheckTerm->getSuccessor(0);
>>>>      IRB.SetInsertPoint(CheckTerm);
>>>> --
>>>> 2.1.0
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140902/7d0fc3ff/attachment.html>