Changeset 252306 in webkit


Ignore:
Timestamp:
Nov 8, 2019 11:05:16 PM (4 years ago)
Author:
Tadeu Zagallo
Message:

[WebAssembly] LLIntGenerator should not retain VirtualRegisters used for constants
https://bugs.webkit.org/show_bug.cgi?id=204028

Reviewed by Yusuke Suzuki.

The LLIntGenerator was keeping track of which RegisterIDs contained constants in order to materialize
the OSR entry data, since constants were not included in the OSR entry buffer. This was originally done
by adding the registers that contained constants to a vector and never reusing them. This is bad because
the bytecode generator reclaims registers by popping unused registers from the end of the vector and
stops as soon as it finds a used register. As it turns out, constants *should* be included in the buffer,
so we don't need to worry about whether registers contain constants and we can just stop retaining the
registers. An assertion was added to doOSREntry to ensure that the size of the scratch buffer matches the
size of the values to be written, which was not true before.
Additionally, add m_constantMap to LLIntGenerator to avoid adding duplicate constants to code blocks.

  • bytecompiler/BytecodeGenerator.h:
  • bytecompiler/BytecodeGeneratorBase.h:
  • wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::addLoop):

  • wasm/WasmLLIntGenerator.cpp:

(JSC::Wasm::LLIntGenerator::addConstant):
(JSC::Wasm::LLIntGenerator::addLoop):

  • wasm/WasmOperations.cpp:

(JSC::Wasm::doOSREntry):

Location:
trunk/Source/JavaScriptCore
Files:
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r252304 r252306  
     12019-11-08  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        [WebAssembly] LLIntGenerator should not retain VirtualRegisters used for constants
     4        https://bugs.webkit.org/show_bug.cgi?id=204028
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        The LLIntGenerator was keeping track of which RegisterIDs contained constants in order to materialize
     9        the OSR entry data, since constants were not included in the OSR entry buffer. This was originally done
     10        by adding the registers that contained constants to a vector and never reusing them. This is bad because
     11        the bytecode generator reclaims registers by popping unused registers from the end of the vector and
     12        stops as soon as it finds a used register. As it turns out, constants *should* be included in the buffer,
     13        so we don't need to worry about whether registers contain constants and we can just stop retaining the
     14        registers. An assertion was added to doOSREntry to ensure that the size of the scratch buffer matches the
     15        size of the values to be written, which was not true before.
     16        Additionally, add m_constantMap to LLIntGenerator to avoid adding duplicate constants to code blocks.
     17
     18        * bytecompiler/BytecodeGenerator.h:
     19        * bytecompiler/BytecodeGeneratorBase.h:
     20        * wasm/WasmB3IRGenerator.cpp:
     21        (JSC::Wasm::B3IRGenerator::addLoop):
     22        * wasm/WasmLLIntGenerator.cpp:
     23        (JSC::Wasm::LLIntGenerator::addConstant):
     24        (JSC::Wasm::LLIntGenerator::addLoop):
     25        * wasm/WasmOperations.cpp:
     26        (JSC::Wasm::doOSREntry):
     27
    1282019-11-08  Yusuke Suzuki  <ysuzuki@apple.com>
    229
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r252273 r252306  
    12261226        SegmentedVector<RegisterID, 32> m_parameters;
    12271227        SegmentedVector<LabelScope, 32> m_labelScopes;
     1228        SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
    12281229        unsigned m_finallyDepth { 0 };
    12291230        unsigned m_localScopeDepth { 0 };
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGeneratorBase.h

    r251886 r252306  
    8686    SegmentedVector<GenericLabel<Traits>, 32> m_labels;
    8787    SegmentedVector<RegisterID, 32> m_calleeLocals;
    88     SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
    8988};
    9089
  • trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

    r251978 r252306  
    13001300            for (unsigned i = 0; i < expressionStack.size(); i++) {
    13011301                auto* value = expressionStack[i];
    1302                 if (value->isConstant())
     1302                if (value->isConstant()) {
     1303                    ++indexInBuffer;
    13031304                    continue;
     1305                }
    13041306
    13051307                if (value->owner != sourceBlock) {
  • trunk/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp

    r252231 r252306  
    4040#include "WasmThunks.h"
    4141#include <wtf/RefPtr.h>
     42#include <wtf/StdUnorderedMap.h>
    4243#include <wtf/Variant.h>
    4344
     
    252253    }
    253254
    254     bool isConstant(RefPtr<RegisterID> reg)
    255     {
    256         VirtualRegister virtualRegister = reg->virtualRegister();
    257         if (virtualRegister.isConstant())
    258             return true;
    259         for (auto& constant : m_constantPoolRegisters) {
    260             if (constant.virtualRegister() == virtualRegister)
    261                 return true;
    262         }
    263         return false;
    264     }
    265 
    266255    struct SwitchEntry {
    267256        InstructionStream::Offset offset;
     
    276265    ExpressionType m_jsNullConstant;
    277266    ExpressionList m_unitializedLocals;
     267    StdUnorderedMap<uint64_t, VirtualRegister> m_constantMap;
    278268};
    279269
     
    512502{
    513503    VirtualRegister source(FirstConstantRegisterIndex + m_codeBlock->m_constants.size());
     504    auto result = m_constantMap.emplace(value, source);
     505    if (result.second)
     506        m_codeBlock->m_constants.append(value);
     507    else
     508        source = result.first->second;
    514509    auto target = newTemporary();
    515     target->ref();
    516     m_constantPoolRegisters.append(target);
    517     m_codeBlock->m_constants.append(value);
    518510    WasmMov::emit(this, target, source);
    519511    return target;
     
    571563    for (unsigned controlIndex = 0; controlIndex < m_parser->controlStack().size(); ++controlIndex) {
    572564        ExpressionList& expressionStack = m_parser->controlStack()[controlIndex].enclosedExpressionStack;
    573         for (auto& expression : expressionStack) {
    574             if (isConstant(expression))
    575                 continue;
     565        for (auto& expression : expressionStack)
    576566            osrEntryData.append(expression->virtualRegister());
    577         }
    578567    }
    579568
  • trunk/Source/JavaScriptCore/wasm/WasmOperations.cpp

    r251690 r252306  
    126126        context.gpr(GPRInfo::argumentGPR0) = 0;
    127127    };
     128
     129    RELEASE_ASSERT(osrEntryCallee.osrEntryScratchBufferSize() == osrEntryData.values().size());
    128130
    129131    uint64_t* buffer = instance->context()->scratchBufferForSize(osrEntryCallee.osrEntryScratchBufferSize());
Note: See TracChangeset for help on using the changeset viewer.