[llvm] r245140 - Accelerate MergeFunctions with hashing
Pete Cooper via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 15 21:40:42 PDT 2015
Thanks for the quick fix. Much appreciated!
Cheers
Pete
Sent from my iPhone
> On Aug 15, 2015, at 9:07 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
>
> It had been failing also there;
> http://bb.pgr.jp/builders/ninja-clang-i686-msc18-R/builds/2679
> http://bb.pgr.jp/builders/ninja-clang-x64-mingw64-RA/builds/7660
>
> Fixed in r245168.
>
>> On Sun, Aug 16, 2015 at 4:48 AM Pete Cooper via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> Hey JF, Jason
>>
>> There’s an ASan/UBSan failure with a mergefunc test on the bots. The blame list range is r245138-r245153.
>>
>> Mind taking a look to see if this commit might be the cause. Its the only MergeFunc related one I could see in the range.
>>
>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/94/consoleFull#-121115394049ba4694-19c4-4d7e-bec5-911270d8a58c
>>
>> Cheers,
>> Pete
>>> On Aug 14, 2015, at 7:06 PM, JF Bastien via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>
>>> I'll defer the answer to Jason, who authored the patch.
>>>
>>> On Aug 14, 2015 6:39 PM, "David Blaikie" <dblaikie at gmail.com> wrote:
>>>>
>>>>
>>>>> On Fri, Aug 14, 2015 at 6:18 PM, JF Bastien via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>>>> Author: jfb
>>>>> Date: Fri Aug 14 20:18:18 2015
>>>>> New Revision: 245140
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=245140&view=rev
>>>>> Log:
>>>>> Accelerate MergeFunctions with hashing
>>>>>
>>>>> This patch makes the Merge Functions pass faster by calculating and comparing
>>>>> a hash value which captures the essential structure of a function before
>>>>> performing a full function comparison.
>>>>>
>>>>> The hash is calculated by hashing the function signature, then walking the basic
>>>>> blocks of the function in the same order as the main comparison function. The
>>>>> opcode of each instruction is hashed in sequence, which means that different
>>>>> functions according to the existing total order cannot have the same hash, as
>>>>> the comparison requires the opcodes of the two functions to be the same order.
>>>>>
>>>>> The hash function is a static member of the FunctionComparator class because it
>>>>> is tightly coupled to the exact comparison function used. For example, functions
>>>>> which are equivalent modulo a single variant callsite might be merged by a more
>>>>> aggressive MergeFunctions, and the hash function would need to be insensitive to
>>>>> these differences in order to exploit this.
>>>>>
>>>>> The hashing function uses a utility class which accumulates the values into an
>>>>> internal state using a standard bit-mixing function. Note that this is a different interface
>>>>> than a regular hashing routine, because the values to be hashed are scattered
>>>>> amongst the properties of a llvm::Function, not linear in memory. This scheme is
>>>>> fast because only one word of state needs to be kept, and the mixing function is
>>>>> a few instructions.
>>>>>
>>>>> The main runOnModule function first computes the hash of each function, and only
>>>>> further processes functions which do not have a unique function hash. The hash
>>>>> is also used to order the sorted function set. If the hashes differ, their
>>>>> values are used to order the functions, otherwise the full comparison is done.
>>>>>
>>>>> Both of these are helpful in speeding up MergeFunctions. Together they result in
>>>>> speedups of 9% for mysqld (a mostly C application with little redundancy), 46%
>>>>> for libxul in Firefox, and 117% for Chromium. (These are all LTO builds.) In all
>>>>> three cases, the new speed of MergeFunctions is about half that of the module
>>>>> verifier, making it relatively inexpensive even for large LTO builds with
>>>>> hundreds of thousands of functions. The same functions are merged, so this
>>>>> change is free performance.
>>>>>
>>>>> Author: jrkoenig
>>>>>
>>>>> Reviewers: nlewycky, dschuff, jfb
>>>>>
>>>>> Subscribers: llvm-commits, aemerson
>>>>>
>>>>> Differential revision: http://reviews.llvm.org/D11923
>>>>>
>>>>> Modified:
>>>>> llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>>>>> llvm/trunk/test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll
>>>>>
>>>>> Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=245140&r1=245139&r2=245140&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
>>>>> +++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Fri Aug 14 20:18:18 2015
>>>>> @@ -27,6 +27,14 @@
>>>>> // -- We define Function* container class with custom "operator<" (FunctionPtr).
>>>>> // -- "FunctionPtr" instances are stored in std::set collection, so every
>>>>> // std::set::insert operation will give you result in log(N) time.
>>>>> +//
>>>>> +// As an optimization, a hash of the function structure is calculated first, and
>>>>> +// two functions are only compared if they have the same hash. This hash is
>>>>> +// cheap to compute, and has the property that if function F == G according to
>>>>> +// the comparison function, then hash(F) == hash(G). This consistency property
>>>>> +// is critical to ensuring all possible merging opportunities are exploited.
>>>>> +// Collisions in the hash affect the speed of the pass but not the correctness
>>>>> +// or determinism of the resulting transformation.
>>>>> //
>>>>> // When a match is found the functions are folded. If both functions are
>>>>> // overridable, we move the functionality into a new internal function and
>>>>> @@ -87,6 +95,7 @@
>>>>> #include "llvm/ADT/STLExtras.h"
>>>>> #include "llvm/ADT/SmallSet.h"
>>>>> #include "llvm/ADT/Statistic.h"
>>>>> +#include "llvm/ADT/Hashing.h"
>>>>> #include "llvm/IR/CallSite.h"
>>>>> #include "llvm/IR/Constants.h"
>>>>> #include "llvm/IR/DataLayout.h"
>>>>> @@ -132,6 +141,10 @@ public:
>>>>>
>>>>> /// Test whether the two functions have equivalent behaviour.
>>>>> int compare();
>>>>> + /// Hash a function. Equivalent functions will have the same hash, and unequal
>>>>> + /// functions will have different hashes with high probability.
>>>>> + typedef uint64_t FunctionHash;
>>>>> + static FunctionHash functionHash(Function &);
>>>>>
>>>>> private:
>>>>> /// Test whether two basic blocks have equivalent behaviour.
>>>>> @@ -390,9 +403,11 @@ private:
>>>>>
>>>>> class FunctionNode {
>>>>> mutable AssertingVH<Function> F;
>>>>> + FunctionComparator::FunctionHash Hash;
>>>>>
>>>>> public:
>>>>> - FunctionNode(Function *F) : F(F) {}
>>>>> + // Note the hash is recalculated potentially multiple times, but it is cheap.
>>>>> + FunctionNode(Function *F) : F(F), Hash(FunctionComparator::functionHash(*F)){}
>>>>> Function *getFunc() const { return F; }
>>>>>
>>>>> /// Replace the reference to the function F by the function G, assuming their
>>>>> @@ -406,6 +421,9 @@ public:
>>>>>
>>>>> void release() { F = 0; }
>>>>> bool operator<(const FunctionNode &RHS) const {
>>>>> + // Order first by hashes, then full function comparison.
>>>>> + if (Hash != RHS.Hash)
>>>>> + return Hash < RHS.Hash;
>>>>> return (FunctionComparator(F, RHS.getFunc()).compare()) == -1;
>>>>> }
>>>>> };
>>>>> @@ -1074,6 +1092,66 @@ int FunctionComparator::compare() {
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +// Accumulate the hash of a sequence of 64-bit integers. This is similar to a
>>>>> +// hash of a sequence of 64bit ints, but the entire input does not need to be
>>>>> +// available at once. This interface is necessary for functionHash because it
>>>>> +// needs to accumulate the hash as the structure of the function is traversed
>>>>> +// without saving these values to an intermediate buffer. This form of hashing
>>>>> +// is not often needed, as usually the object to hash is just read from a
>>>>> +// buffer.
>>>>> +class HashAccumulator64 {
>>>>
>>>> Any chance of using LLVM's Hashing.h for hash aggregation? (it's already got accumulator support, if I'm not mistaken?)
>>>>
>>>>> + uint64_t Hash;
>>>>> +public:
>>>>> + // Initialize to random constant, so the state isn't zero.
>>>>> + HashAccumulator64() { Hash = 0x6acaa36bef8325c5ULL; }
>>>>> + void add(uint64_t V) {
>>>>> + Hash = llvm::hashing::detail::hash_16_bytes(Hash, V);
>>>>> + }
>>>>> + // No finishing is required, because the entire hash value is used.
>>>>> + uint64_t getHash() { return Hash; }
>>>>> +};
>>>>> +
>>>>> +// A function hash is calculated by considering only the number of arguments and
>>>>> +// whether a function is varargs, the order of basic blocks (given by the
>>>>> +// successors of each basic block in depth first order), and the order of
>>>>> +// opcodes of each instruction within each of these basic blocks. This mirrors
>>>>> +// the strategy compare() uses to compare functions by walking the BBs in depth
>>>>> +// first order and comparing each instruction in sequence. Because this hash
>>>>> +// does not look at the operands, it is insensitive to things such as the
>>>>> +// target of calls and the constants used in the function, which makes it useful
>>>>> +// when possibly merging functions which are the same modulo constants and call
>>>>> +// targets.
>>>>> +FunctionComparator::FunctionHash FunctionComparator::functionHash(Function &F) {
>>>>> + HashAccumulator64 H;
>>>>> + H.add(F.isVarArg());
>>>>> + H.add(F.arg_size());
>>>>> +
>>>>> + SmallVector<const BasicBlock *, 8> BBs;
>>>>> + SmallSet<const BasicBlock *, 16> VisitedBBs;
>>>>> +
>>>>> + // Walk the blocks in the same order as FunctionComparator::compare(),
>>>>> + // accumulating the hash of the function "structure." (BB and opcode sequence)
>>>>> + BBs.push_back(&F.getEntryBlock());
>>>>> + VisitedBBs.insert(BBs[0]);
>>>>> + while (!BBs.empty()) {
>>>>> + const BasicBlock *BB = BBs.pop_back_val();
>>>>> + // This random value acts as a block header, as otherwise the partition of
>>>>> + // opcodes into BBs wouldn't affect the hash, only the order of the opcodes
>>>>> + H.add(45798);
>>>>> + for (auto &Inst : *BB) {
>>>>> + H.add(Inst.getOpcode());
>>>>> + }
>>>>> + const TerminatorInst *Term = BB->getTerminator();
>>>>> + for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
>>>>> + if (!VisitedBBs.insert(Term->getSuccessor(i)).second)
>>>>> + continue;
>>>>> + BBs.push_back(Term->getSuccessor(i));
>>>>> + }
>>>>> + }
>>>>> + return H.getHash();
>>>>> +}
>>>>> +
>>>>> +
>>>>> namespace {
>>>>>
>>>>> /// MergeFunctions finds functions which will generate identical machine code,
>>>>> @@ -1228,11 +1306,28 @@ bool MergeFunctions::doSanityCheck(std::
>>>>> bool MergeFunctions::runOnModule(Module &M) {
>>>>> bool Changed = false;
>>>>>
>>>>> - for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) {
>>>>> - if (!I->isDeclaration() && !I->hasAvailableExternallyLinkage())
>>>>> - Deferred.push_back(WeakVH(I));
>>>>> + // All functions in the module, ordered by hash. Functions with a unique
>>>>> + // hash value are easily eliminated.
>>>>> + std::vector<std::pair<FunctionComparator::FunctionHash, Function *>>
>>>>> + HashedFuncs;
>>>>> + for (Function &Func : M) {
>>>>> + if (!Func.isDeclaration() && !Func.hasAvailableExternallyLinkage()) {
>>>>> + HashedFuncs.push_back({FunctionComparator::functionHash(Func), &Func});
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + std::sort(HashedFuncs.begin(), HashedFuncs.end());
>>>>> +
>>>>> + auto S = HashedFuncs.begin();
>>>>> + for (auto I = HashedFuncs.begin(), IE = HashedFuncs.end(); I != IE; ++I) {
>>>>> + // If the hash value matches the previous value or the next one, we must
>>>>> + // consider merging it. Otherwise it is dropped and never considered again.
>>>>> + if ((I != S && std::prev(I)->first == I->first) ||
>>>>> + (std::next(I) != IE && std::next(I)->first == I->first) ) {
>>>>> + Deferred.push_back(WeakVH(I->second));
>>>>> + }
>>>>> }
>>>>> -
>>>>> +
>>>>> do {
>>>>> std::vector<WeakVH> Worklist;
>>>>> Deferred.swap(Worklist);
>>>>>
>>>>> Modified: llvm/trunk/test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll?rev=245140&r1=245139&r2=245140&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll (original)
>>>>> +++ llvm/trunk/test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll Fri Aug 14 20:18:18 2015
>>>>> @@ -63,14 +63,6 @@ lpad:
>>>>> resume { i8*, i32 } zeroinitializer
>>>>> }
>>>>>
>>>>> -define i8 @call_with_same_range() {
>>>>> -; CHECK-LABEL: @call_with_same_range
>>>>> -; CHECK: tail call i8 @call_with_range
>>>>> - bitcast i8 0 to i8
>>>>> - %out = call i8 @dummy(), !range !0
>>>>> - ret i8 %out
>>>>> -}
>>>>> -
>>>>> define i8 @invoke_with_same_range() personality i8* undef {
>>>>> ; CHECK-LABEL: @invoke_with_same_range()
>>>>> ; CHECK: tail call i8 @invoke_with_range()
>>>>> @@ -84,6 +76,16 @@ lpad:
>>>>> resume { i8*, i32 } zeroinitializer
>>>>> }
>>>>>
>>>>> +define i8 @call_with_same_range() {
>>>>> +; CHECK-LABEL: @call_with_same_range
>>>>> +; CHECK: tail call i8 @call_with_range
>>>>> + bitcast i8 0 to i8
>>>>> + %out = call i8 @dummy(), !range !0
>>>>> + ret i8 %out
>>>>> +}
>>>>> +
>>>>> +
>>>>> +
>>>>> declare i8 @dummy();
>>>>> declare i32 @__gxx_personality_v0(...)
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150815/6ebd431f/attachment.html>
More information about the llvm-commits
mailing list