Hi, The change to add weights is trivial. > I am not against it and the patch is welcome, but it will need to be > accompanied with SPEC performance numbers.
I modified ASan and did a quick run on SPEC. Results are available in this spreadsheet [1]. Detailed output from "perf stat", including standard deviations from three runs, is available at [2]. Overall, it seems that branch weights reduce the runtime of SPEC benchmarks by about 3% on average, and reduce branch misses by about 8%. Results vary quite strongly between benchmarks, but branch weights seem to be beneficial for 14 out of 16. This uses just the SPEC "test" workload, so the results might not be very precise. I'll re-run with longer workloads to confirm. A patch is attached. Cheers, Jonas [1]: https://docs.google.com/spreadsheets/d/1t5sMnBMXWr5BXM8jZiW_G2XSTtVczaOrPyPAEMXL9qc/ [2]: https://drive.google.com/open?id=0B8pYrLJnKwDSa3lQaXdudGdrNFU&authuser=0 -- You received this message because you are subscribed to the Google Groups "address-sanitizer" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
From 95e8fee003f1a8c7c575046b251d83bba86def3a Mon Sep 17 00:00:00 2001 From: Jonas Wagner <[email protected]> Date: Thu, 28 Aug 2014 14:19:55 +0200 Subject: [PATCH] Assign a low branch weight to ASan's slow path. 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 8d21108..547eb9c 100644 --- a/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -825,8 +825,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
