Changeset 204065 in webkit


Ignore:
Timestamp:
Aug 2, 2016 8:45:07 PM (8 years ago)
Author:
benjamin@webkit.org
Message:

[JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
​https://bugs.webkit.org/show_bug.cgi?id=160370

Reviewed by Saam Barati.

Source/JavaScriptCore:

We use a ton of AbstractValue to run the Abstract Interpreter.

When we set up the initial values, the compiler sets
a zero on a first word, a one on a second word, and a zero
again on a third word.
Since no vector or double-store can deal with 3 words, unrolling
is done by repeating those instructions.

The reason for the one was TinyPtrSet. It needed a flag for
empty value to identify the set as thin. I flipped the flag to "fat"
to make sure TinyPtrSet is initialized to zero.

With that done, I just had to clean some places to make
the initialization shorter.
It makes the binary easier to follow but this does not help with
the bigger problem: the time spent per block on Abstract Interpreter.

  • bytecode/Operands.h:

The traits were useless, no client code defines it.

(JSC::Operands::Operands):
(JSC::Operands::ensureLocals):
Because of the size of the function, llvm is not inlining it.
We were literally loading 3 registers from memory and storing
them in the vector.
Now that AbstractValue has a VectorTraits, we should just rely
on the memset of Vector when possible.

(JSC::Operands::getLocal):
(JSC::Operands::setArgumentFirstTime):
(JSC::Operands::setLocalFirstTime):
(JSC::Operands::clear):
(JSC::OperandValueTraits::defaultValue): Deleted.
(JSC::OperandValueTraits::isEmptyForDump): Deleted.

  • bytecode/OperandsInlines.h:

(JSC::Operands<T>::dumpInContext):
(JSC::Operands<T>::dump):
(JSC::Traits>::dumpInContext): Deleted.
(JSC::Traits>::dump): Deleted.

  • dfg/DFGAbstractValue.cpp:
  • dfg/DFGAbstractValue.h:

(JSC::DFG::AbstractValue::AbstractValue):

Source/WTF:

  • wtf/TinyPtrSet.h:

(WTF::TinyPtrSet::isThin):
(WTF::TinyPtrSet::set):

Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r204058 r204065  
     12016-08-02  Benjamin Poulain  <benjamin@webkit.org>
     2
     3        [JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
     4        https://bugs.webkit.org/show_bug.cgi?id=160370
     5
     6        Reviewed by Saam Barati.
     7
     8        We use a ton of AbstractValue to run the Abstract Interpreter.
     9
     10        When we set up the initial values, the compiler sets
     11        a zero on a first word, a one on a second word, and a zero
     12        again on a third word.
     13        Since no vector or double-store can deal with 3 words, unrolling
     14        is done by repeating those instructions.
     15
     16        The reason for the one was TinyPtrSet. It needed a flag for
     17        empty value to identify the set as thin. I flipped the flag to "fat"
     18        to make sure TinyPtrSet is initialized to zero.
     19
     20        With that done, I just had to clean some places to make
     21        the initialization shorter.
     22        It makes the binary easier to follow but this does not help with
     23        the bigger problem: the time spent per block on Abstract Interpreter.
     24
     25        * bytecode/Operands.h:
     26        The traits were useless, no client code defines it.
     27
     28        (JSC::Operands::Operands):
     29        (JSC::Operands::ensureLocals):
     30        Because of the size of the function, llvm is not inlining it.
     31        We were literally loading 3 registers from memory and storing
     32        them in the vector.
     33        Now that AbstractValue has a VectorTraits, we should just rely
     34        on the memset of Vector when possible.
     35
     36        (JSC::Operands::getLocal):
     37        (JSC::Operands::setArgumentFirstTime):
     38        (JSC::Operands::setLocalFirstTime):
     39        (JSC::Operands::clear):
     40        (JSC::OperandValueTraits::defaultValue): Deleted.
     41        (JSC::OperandValueTraits::isEmptyForDump): Deleted.
     42        * bytecode/OperandsInlines.h:
     43        (JSC::Operands<T>::dumpInContext):
     44        (JSC::Operands<T>::dump):
     45        (JSC::Traits>::dumpInContext): Deleted.
     46        (JSC::Traits>::dump): Deleted.
     47        * dfg/DFGAbstractValue.cpp:
     48        * dfg/DFGAbstractValue.h:
     49        (JSC::DFG::AbstractValue::AbstractValue):
     50
    1512016-08-02  Saam Barati  <sbarati@apple.com>
    252

  • trunk/Source/JavaScriptCore/bytecode/Operands.h

    r186691 r204065  
    11/*
    2  * Copyright (C) 2011, 2012, 2013, 2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011, 2012, 2013, 2015, 2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3838template<typename T> struct OperandValueTraits;
    3939
     40enum OperandKind { ArgumentOperand, LocalOperand };
     41
     42enum OperandsLikeTag { OperandsLike };
     43
    4044template<typename T>
    41 struct OperandValueTraits {
    42     static T defaultValue() { return T(); }
    43     static bool isEmptyForDump(const T& value) { return !value; }
    44 };
    45 
    46 enum OperandKind { ArgumentOperand, LocalOperand };
    47 
    48 enum OperandsLikeTag { OperandsLike };
    49 
    50 template<typename T, typename Traits = OperandValueTraits<T>>
    5145class Operands {
    5246public:
    5347    Operands() { }
    5448   
    55     explicit Operands(size_t numArguments, size_t numLocals, const T& initialValue = Traits::defaultValue())
     49    explicit Operands(size_t numArguments, size_t numLocals)
     50    {
     51        if (WTF::VectorTraits<T>::needsInitialization) {
     52            m_arguments.resize(numArguments);
     53            m_locals.resize(numLocals);
     54        } else {
     55            m_arguments.fill(T(), numArguments);
     56            m_locals.fill(T(), numLocals);
     57        }
     58    }
     59
     60    explicit Operands(size_t numArguments, size_t numLocals, const T& initialValue)
    5661    {
    5762        m_arguments.fill(initialValue, numArguments);
    … …  
    5964    }
    6065   
    61     template<typename U, typename OtherTraits>
    62     explicit Operands(OperandsLikeTag, const Operands<U, OtherTraits>& other)
    63     {
    64         m_arguments.fill(Traits::defaultValue(), other.numberOfArguments());
    65         m_locals.fill(Traits::defaultValue(), other.numberOfLocals());
     66    template<typename U>
     67    explicit Operands(OperandsLikeTag, const Operands<U>& other)
     68    {
     69        m_arguments.fill(T(), other.numberOfArguments());
     70        m_locals.fill(T(), other.numberOfLocals());
    6671    }
    6772   
    … …  
    97102    }
    98103   
    99     void ensureLocals(size_t size, const T& ensuredValue = Traits::defaultValue())
     104    void ensureLocals(size_t size)
     105    {
     106        if (size <= m_locals.size())
     107            return;
     108
     109        size_t oldSize = m_locals.size();
     110        m_locals.resize(size);
     111        if (!WTF::VectorTraits<T>::needsInitialization) {
     112            for (size_t i = oldSize; i < m_locals.size(); ++i)
     113                m_locals[i] = T();
     114        }
     115    }
     116
     117    void ensureLocals(size_t size, const T& ensuredValue)
    100118    {
    101119        if (size <= m_locals.size())
    … …  
    118136    {
    119137        if (idx >= m_locals.size())
    120             return Traits::defaultValue();
     138            return T();
    121139        return m_locals[idx];
    122140    }
    … …  
    124142    void setArgumentFirstTime(size_t idx, const T& value)
    125143    {
    126         ASSERT(m_arguments[idx] == Traits::defaultValue());
     144        ASSERT(m_arguments[idx] == T());
    127145        argument(idx) = value;
    128146    }
    … …  
    130148    void setLocalFirstTime(size_t idx, const T& value)
    131149    {
    132         ASSERT(idx >= m_locals.size() || m_locals[idx] == Traits::defaultValue());
     150        ASSERT(idx >= m_locals.size() || m_locals[idx] == T());
    133151        setLocal(idx, value);
    134152    }
    … …  
    246264    void clear()
    247265    {
    248         fill(Traits::defaultValue());
     266        fill(T());
    249267    }
    250268   

  • trunk/Source/JavaScriptCore/bytecode/OperandsInlines.h

    r174318 r204065  
    11/*
    2  * Copyright (C) 2013 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3232namespace JSC {
    3333
    34 template<typename T, typename Traits>
    35 void Operands<T, Traits>::dumpInContext(PrintStream& out, DumpContext* context) const
     34template<typename T>
     35void Operands<T>::dumpInContext(PrintStream& out, DumpContext* context) const
    3636{
    3737    CommaPrinter comma(" ");
    3838    for (size_t argumentIndex = numberOfArguments(); argumentIndex--;) {
    39         if (Traits::isEmptyForDump(argument(argumentIndex)))
     39        if (!argument(argumentIndex))
    4040            continue;
    4141        out.print(comma, "arg", argumentIndex, ":", inContext(argument(argumentIndex), context));
    4242    }
    4343    for (size_t localIndex = 0; localIndex < numberOfLocals(); ++localIndex) {
    44         if (Traits::isEmptyForDump(local(localIndex)))
     44        if (!local(localIndex))
    4545            continue;
    4646        out.print(comma, "loc", localIndex, ":", inContext(local(localIndex), context));
    … …  
    4848}
    4949
    50 template<typename T, typename Traits>
    51 void Operands<T, Traits>::dump(PrintStream& out) const
     50template<typename T>
     51void Operands<T>::dump(PrintStream& out) const
    5252{
    5353    CommaPrinter comma(" ");
    5454    for (size_t argumentIndex = numberOfArguments(); argumentIndex--;) {
    55         if (Traits::isEmptyForDump(argument(argumentIndex)))
     55        if (!argument(argumentIndex))
    5656            continue;
    5757        out.print(comma, "arg", argumentIndex, ":", argument(argumentIndex));
    5858    }
    5959    for (size_t localIndex = 0; localIndex < numberOfLocals(); ++localIndex) {
    60         if (Traits::isEmptyForDump(local(localIndex)))
     60        if (!local(localIndex))
    6161            continue;
    6262        out.print(comma, "loc", localIndex, ":", local(localIndex));

  • trunk/Source/JavaScriptCore/dfg/DFGAbstractValue.cpp

    r200430 r204065  
    3535namespace JSC { namespace DFG {
    3636
     37static_assert(sizeof(AbstractValue) == (sizeof(void*) + sizeof(unsigned) * 2 + sizeof(JSValue)), "AbstractValue should be as small as possible.");
     38
    3739void AbstractValue::observeTransitions(const TransitionVector& vector)
    3840{

  • trunk/Source/JavaScriptCore/dfg/DFGAbstractValue.h

    r200034 r204065  
    11/*
    2  * Copyright (C) 2011-2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    5656        , m_arrayModes(0)
    5757    {
     58#ifndef NDEBUG
     59        // The WTF Traits for AbstractValue allow the initialization of values with bzero().
     60        // We verify the correctness of this assumption here.
     61        static bool needsDefaultConstructorCheck = true;
     62        if (needsDefaultConstructorCheck) {
     63            needsDefaultConstructorCheck = false;
     64
     65            for (unsigned i = 0; i < sizeof(AbstractValue); ++i)
     66                ASSERT(!(reinterpret_cast<char*>(this)[i]));
     67        }
     68#endif
    5869    }
    5970   
    … …  
    457468} } // namespace JSC::DFG
    458469
     470namespace WTF {
     471template <>
     472struct VectorTraits<JSC::DFG::AbstractValue> : VectorTraitsBase<false, JSC::DFG::AbstractValue> {
     473    static const bool canInitializeWithMemset = true;
     474};
     475
     476template <>
     477struct HashTraits<JSC::DFG::AbstractValue> : GenericHashTraits<JSC::DFG::AbstractValue> {
     478    static const bool emptyValueIsZero = true;
     479};
     480};
     481
    459482#endif // ENABLE(DFG_JIT)
    460483

  • trunk/Source/WTF/ChangeLog

    r204061 r204065  
     12016-08-02  Benjamin Poulain  <benjamin@webkit.org>
     2
     3        [JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
     4        https://bugs.webkit.org/show_bug.cgi?id=160370
     5
     6        Reviewed by Saam Barati.
     7
     8        * wtf/TinyPtrSet.h:
     9        (WTF::TinyPtrSet::isThin):
     10        (WTF::TinyPtrSet::set):
     11
    1122016-08-02  Benjamin Poulain  <bpoulain@apple.com>
    213

  • trunk/Source/WTF/wtf/TinyPtrSet.h

    r199382 r204065  
    11/*
    2  * Copyright (C) 2015 Apple Inc. All rights reserved.
     2 * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    372372    friend class JSC::DFG::StructureAbstractValue;
    373373
    374     static const uintptr_t thinFlag = 1;
     374    static const uintptr_t fatFlag = 1;
    375375    static const uintptr_t reservedFlag = 2;
    376     static const uintptr_t flags = thinFlag | reservedFlag;
     376    static const uintptr_t flags = fatFlag | reservedFlag;
    377377    static const uintptr_t reservedValue = 4;
    378378
    … …  
    464464    }
    465465   
    466     bool isThin() const { return m_pointer & thinFlag; }
     466    bool isThin() const { return !(m_pointer & fatFlag); }
    467467   
    468468    void* pointer() const
    … …  
    497497    void set(uintptr_t pointer, bool singleEntry)
    498498    {
    499         m_pointer = pointer | (singleEntry ? thinFlag : 0) | (m_pointer & reservedFlag);
     499        m_pointer = pointer | (singleEntry ? 0 : fatFlag) | (m_pointer & reservedFlag);
    500500    }
    501501    bool getReservedFlag() const { return m_pointer & reservedFlag; }
Note: See TracChangeset for help on using the changeset viewer.