Changeset 262570 in webkit


Ignore:
Timestamp:
Jun 4, 2020, 2:07:42 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

Add Options::validateDoesGC() for turning DoesGC validation on/off.
​https://bugs.webkit.org/show_bug.cgi?id=212773

Reviewed by Saam Barati.

It will default to on if ASSERT_ENABLED because we want testing to be done with
the validation on. When needed, we can turn it off if we need to e.g. to
de-clutter disassembly dumps while debugging.

If Options::validateDoesGC() is false, we turn off JIT code emission for this
check, as well as skip the validation checks. There are still places in C++
code that store to DoesGC::m_value without checking Options::validateDoesGC().
It doesn't hurt to just let these stores proceed, and performance-wise, it's
probably cheaper to just do the store unconditionally than to gate it on a load of
Options::validateDoesGC() first.

Also made it explicit that the check on validateDFGDoesGC is a constexpr check.

  • dfg/DFGDoesGCCheck.cpp:

(JSC::DFG::DoesGCCheck::verifyCanGC):

  • dfg/DFGOSRExit.cpp:

(JSC::DFG::OSRExit::compileExit):

  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):

  • ftl/FTLOSRExitCompiler.cpp:

(JSC::FTL::compileStub):

  • runtime/OptionsList.h:
Location:
trunk/Source/JavaScriptCore
Files:
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r262568 r262570  
     12020-06-04  Mark Lam  <mark.lam@apple.com>
     2
     3        Add Options::validateDoesGC() for turning DoesGC validation on/off.
     4        https://bugs.webkit.org/show_bug.cgi?id=212773
     5
     6        Reviewed by Saam Barati.
     7
     8        It will default to on if ASSERT_ENABLED because we want testing to be done with
     9        the validation on.  When needed, we can turn it off if we need to e.g. to
     10        de-clutter disassembly dumps while debugging.
     11
     12        If Options::validateDoesGC() is false, we turn off JIT code emission for this
     13        check, as well as skip the validation checks.  There are still places in C++
     14        code that store to DoesGC::m_value without checking Options::validateDoesGC().
     15        It doesn't hurt to just let these stores proceed, and performance-wise, it's
     16        probably cheaper to just do the store unconditionally than to gate it on a load of
     17        Options::validateDoesGC() first.
     18
     19        Also made it explicit that the check on validateDFGDoesGC is a constexpr check.
     20
     21        * dfg/DFGDoesGCCheck.cpp:
     22        (JSC::DFG::DoesGCCheck::verifyCanGC):
     23        * dfg/DFGOSRExit.cpp:
     24        (JSC::DFG::OSRExit::compileExit):
     25        * dfg/DFGSpeculativeJIT32_64.cpp:
     26        (JSC::DFG::SpeculativeJIT::compile):
     27        * dfg/DFGSpeculativeJIT64.cpp:
     28        (JSC::DFG::SpeculativeJIT::compile):
     29        * ftl/FTLLowerDFGToB3.cpp:
     30        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
     31        * ftl/FTLOSRExitCompiler.cpp:
     32        (JSC::FTL::compileStub):
     33        * runtime/OptionsList.h:
     34
    1352020-06-04  Ross Kirsling  <ross.kirsling@sony.com>
    236

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

    r262562 r262570  
    3131#include "DFGNodeType.h"
    3232#include "Heap.h"
     33#include "Options.h"
    3334#include "VMInspector.h"
    3435#include <wtf/DataLog.h>
    … …  
    4647    // in the header file.
    4748    static_assert(numberOfNodeTypes <= (1 << nodeOpBits));
     49
     50    if (!Options::validateDoesGC())
     51        return;
    4852
    4953    if (!expectDoesGC()) {

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

    r262562 r262570  
    144144    auto scope = DECLARE_THROW_SCOPE(vm);
    145145
    146     if (validateDFGDoesGC) {
     146    if constexpr (validateDFGDoesGC) {
    147147        // We're about to exit optimized code. So, there's no longer any optimized
    148148        // code running that expects no GC.
    … …  
    552552
    553553#if USE(JSVALUE64)
    554     if (validateDFGDoesGC) {
    555         // We're about to exit optimized code. So, there's no longer any optimized
    556         // code running that expects no GC. We need to set this before arguments
    557         // materialization below (see emitRestoreArguments()).
    558 
    559         // Even though we set Heap::m_doesGC in compileOSRExit(), we also need
    560         // to set it here because compileOSRExit() is only called on the first time
    561         // we exit from this site, but all subsequent exits will take this compiled
    562         // ramp without calling compileOSRExit() first.
    563         jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::DFGOSRExit)), vm.heap.addressOfDoesGC());
     554    if constexpr (validateDFGDoesGC) {
     555        if (Options::validateDoesGC()) {
     556            // We're about to exit optimized code. So, there's no longer any optimized
     557            // code running that expects no GC. We need to set this before arguments
     558            // materialization below (see emitRestoreArguments()).
     559
     560            // Even though we set Heap::m_doesGC in compileOSRExit(), we also need
     561            // to set it here because compileOSRExit() is only called on the first time
     562            // we exit from this site, but all subsequent exits will take this compiled
     563            // ramp without calling compileOSRExit() first.
     564            jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::DFGOSRExit)), vm.heap.addressOfDoesGC());
     565        }
    564566    }
    565567#endif

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

    r262562 r262570  
    18081808    NodeType op = node->op();
    18091809
    1810     if (validateDFGDoesGC) {
    1811         bool expectDoesGC = doesGC(m_jit.graph(), node);
    1812         m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
     1810    if constexpr (validateDFGDoesGC) {
     1811        if (Options::validateDoesGC()) {
     1812            bool expectDoesGC = doesGC(m_jit.graph(), node);
     1813            m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
     1814        }
    18131815    }
    18141816

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

    r262562 r262570  
    21372137    NodeType op = node->op();
    21382138
    2139     if (validateDFGDoesGC) {
    2140         bool expectDoesGC = doesGC(m_jit.graph(), node);
    2141         m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
     2139    if constexpr (validateDFGDoesGC) {
     2140        if (Options::validateDoesGC()) {
     2141            bool expectDoesGC = doesGC(m_jit.graph(), node);
     2142            m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
     2143        }
    21422144    }
    21432145

  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r262562 r262570  
    703703            validateAIState(m_node);
    704704
    705         if (validateDFGDoesGC) {
    706             bool expectDoesGC = doesGC(m_graph, m_node);
    707             m_out.store(m_out.constInt32(DoesGCCheck::encode(expectDoesGC, m_node->index(), m_node->op())), m_out.absolute(vm().heap.addressOfDoesGC()));
     705        if constexpr (validateDFGDoesGC) {
     706            if (Options::validateDoesGC()) {
     707                bool expectDoesGC = doesGC(m_graph, m_node);
     708                m_out.store(m_out.constInt32(DoesGCCheck::encode(expectDoesGC, m_node->index(), m_node->op())), m_out.absolute(vm().heap.addressOfDoesGC()));
     709            }
    708710        }
    709711

  • trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

    r262562 r262570  
    207207    saveAllRegisters(jit, registerScratch);
    208208   
    209     if (validateDFGDoesGC) {
    210         // We're about to exit optimized code. So, there's no longer any optimized
    211         // code running that expects no GC. We need to set this before object
    212         // materialization below.
    213 
    214         // Even though we set Heap::m_doesGC in compileFTLOSRExit(), we also need
    215         // to set it here because compileFTLOSRExit() is only called on the first time
    216         // we exit from this site, but all subsequent exits will take this compiled
    217         // ramp without calling compileFTLOSRExit() first.
    218         jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::FTLOSRExit)), vm.heap.addressOfDoesGC());
     209    if constexpr (validateDFGDoesGC) {
     210        if (Options::validateDoesGC()) {
     211            // We're about to exit optimized code. So, there's no longer any optimized
     212            // code running that expects no GC. We need to set this before object
     213            // materialization below.
     214
     215            // Even though we set Heap::m_doesGC in compileFTLOSRExit(), we also need
     216            // to set it here because compileFTLOSRExit() is only called on the first time
     217            // we exit from this site, but all subsequent exits will take this compiled
     218            // ramp without calling compileFTLOSRExit() first.
     219            jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::FTLOSRExit)), vm.heap.addressOfDoesGC());
     220        }
    219221    }
    220222
    … …  
    546548    VM& vm = callFrame->deprecatedVM();
    547549
    548     if (validateDFGDoesGC) {
     550    if constexpr (validateDFGDoesGC) {
    549551        // We're about to exit optimized code. So, there's no longer any optimized
    550552        // code running that expects no GC.

  • trunk/Source/JavaScriptCore/heap/CompleteSubspace.cpp

    r262513 r262570  
    120120void* CompleteSubspace::tryAllocateSlow(VM& vm, size_t size, GCDeferralContext* deferralContext)
    121121{
    122     if (validateDFGDoesGC)
     122    if constexpr (validateDFGDoesGC)
    123123        vm.heap.verifyCanGC();
    124124
    … …  
    156156void* CompleteSubspace::reallocatePreciseAllocationNonVirtual(VM& vm, HeapCell* oldCell, size_t size, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
    157157{
    158     if (validateDFGDoesGC)
     158    if constexpr (validateDFGDoesGC)
    159159        vm.heap.verifyCanGC();
    160160

  • trunk/Source/JavaScriptCore/heap/CompleteSubspaceInlines.h

    r262513 r262570  
    3333ALWAYS_INLINE void* CompleteSubspace::allocateNonVirtual(VM& vm, size_t size, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
    3434{
    35     if (validateDFGDoesGC)
     35    if constexpr (validateDFGDoesGC)
    3636        vm.heap.verifyCanGC();
    3737

  • trunk/Source/JavaScriptCore/heap/DeferGC.h

    r262513 r262570  
    4545    ~DeferGC()
    4646    {
    47         if (validateDFGDoesGC)
     47        if constexpr (validateDFGDoesGC)
    4848            m_heap.verifyCanGC();
    4949        m_heap.decrementDeferralDepthAndGCIfNeeded();

  • trunk/Source/JavaScriptCore/heap/GCDeferralContextInlines.h

    r262513 r262570  
    3838ALWAYS_INLINE GCDeferralContext::~GCDeferralContext()
    3939{
    40     if (validateDFGDoesGC)
     40    if constexpr (validateDFGDoesGC)
    4141        m_heap.verifyCanGC();
    4242

  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r262513 r262570  
    10641064void Heap::collectNow(Synchronousness synchronousness, GCRequest request)
    10651065{
    1066     if (validateDFGDoesGC)
     1066    if constexpr (validateDFGDoesGC)
    10671067        verifyCanGC();
    10681068
    … …  
    10971097void Heap::collectAsync(GCRequest request)
    10981098{
    1099     if (validateDFGDoesGC)
     1099    if constexpr (validateDFGDoesGC)
    11001100        verifyCanGC();
    11011101
    … …  
    11211121void Heap::collectSync(GCRequest request)
    11221122{
    1123     if (validateDFGDoesGC)
     1123    if constexpr (validateDFGDoesGC)
    11241124        verifyCanGC();
    11251125
    … …  
    17841784void Heap::stopIfNecessarySlow()
    17851785{
    1786     if (validateDFGDoesGC)
     1786    if constexpr (validateDFGDoesGC)
    17871787        verifyCanGC();
    17881788
    … …  
    17991799bool Heap::stopIfNecessarySlow(unsigned oldState)
    18001800{
    1801     if (validateDFGDoesGC)
     1801    if constexpr (validateDFGDoesGC)
    18021802        verifyCanGC();
    18031803
    … …  
    26012601{
    26022602    ASSERT(deferralContext || isDeferred() || !DisallowGC::isInEffectOnCurrentThread());
    2603     if (validateDFGDoesGC)
     2603    if constexpr (validateDFGDoesGC)
    26042604        verifyCanGC();
    26052605

  • trunk/Source/JavaScriptCore/heap/HeapInlines.h

    r262513 r262570  
    11/*
    2  * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2020 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    236236inline void Heap::acquireAccess()
    237237{
    238     if (validateDFGDoesGC)
     238    if constexpr (validateDFGDoesGC)
    239239        verifyCanGC();
    240240
    … …  
    263263inline void Heap::stopIfNecessary()
    264264{
    265     if (validateDFGDoesGC)
     265    if constexpr (validateDFGDoesGC)
    266266        verifyCanGC();
    267267

  • trunk/Source/JavaScriptCore/heap/LocalAllocatorInlines.h

    r262513 r262570  
    11/*
    2  * Copyright (C) 2018-2019 Apple Inc. All rights reserved.
     2 * Copyright (C) 2018-2020 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3333ALWAYS_INLINE void* LocalAllocator::allocate(Heap& heap, GCDeferralContext* deferralContext, AllocationFailureMode failureMode)
    3434{
    35     if (validateDFGDoesGC)
     35    if constexpr (validateDFGDoesGC)
    3636        heap.verifyCanGC();
    3737    return m_freeList.allocate(

  • trunk/Source/JavaScriptCore/heap/PreciseAllocation.cpp

    r262513 r262570  
    11/*
    2  * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
     2 * Copyright (C) 2016-2020 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    4343PreciseAllocation* PreciseAllocation::tryCreate(Heap& heap, size_t size, Subspace* subspace, unsigned indexInSpace)
    4444{
    45     if (validateDFGDoesGC)
     45    if constexpr (validateDFGDoesGC)
    4646        heap.verifyCanGC();
    4747
    … …  
    123123PreciseAllocation* PreciseAllocation::createForLowerTier(Heap& heap, size_t size, Subspace* subspace, uint8_t lowerTierIndex)
    124124{
    125     if (validateDFGDoesGC)
     125    if constexpr (validateDFGDoesGC)
    126126        heap.verifyCanGC();
    127127

  • trunk/Source/JavaScriptCore/runtime/JSString.h

    r262513 r262570  
    733733ALWAYS_INLINE JSString* jsSingleCharacterString(VM& vm, UChar c)
    734734{
    735     if (validateDFGDoesGC)
     735    if constexpr (validateDFGDoesGC)
    736736        vm.heap.verifyCanGC();
    737737    if (c <= maxSingleCharacterString)
    … …  
    763763ALWAYS_INLINE AtomString JSString::toAtomString(JSGlobalObject* globalObject) const
    764764{
    765     if (validateDFGDoesGC)
     765    if constexpr (validateDFGDoesGC)
    766766        vm().heap.verifyCanGC();
    767767    if (isRope())
    … …  
    772772ALWAYS_INLINE RefPtr<AtomStringImpl> JSString::toExistingAtomString(JSGlobalObject* globalObject) const
    773773{
    774     if (validateDFGDoesGC)
     774    if constexpr (validateDFGDoesGC)
    775775        vm().heap.verifyCanGC();
    776776    if (isRope())
    … …  
    783783inline const String& JSString::value(JSGlobalObject* globalObject) const
    784784{
    785     if (validateDFGDoesGC)
     785    if constexpr (validateDFGDoesGC)
    786786        vm().heap.verifyCanGC();
    787787    if (isRope())
    … …  
    793793{
    794794    if (allocationAllowed) {
    795         if (validateDFGDoesGC)
     795        if constexpr (validateDFGDoesGC)
    796796            vm().heap.verifyCanGC();
    797797        if (isRope()) {
    … …  
    983983ALWAYS_INLINE StringView JSRopeString::unsafeView(JSGlobalObject* globalObject) const
    984984{
    985     if (validateDFGDoesGC)
     985    if constexpr (validateDFGDoesGC)
    986986        vm().heap.verifyCanGC();
    987987    if (isSubstring()) {
    … …  
    996996ALWAYS_INLINE StringViewWithUnderlyingString JSRopeString::viewWithUnderlyingString(JSGlobalObject* globalObject) const
    997997{
    998     if (validateDFGDoesGC)
     998    if constexpr (validateDFGDoesGC)
    999999        vm().heap.verifyCanGC();
    10001000    if (isSubstring()) {
    … …  
    10101010ALWAYS_INLINE StringView JSString::unsafeView(JSGlobalObject* globalObject) const
    10111011{
    1012     if (validateDFGDoesGC)
     1012    if constexpr (validateDFGDoesGC)
    10131013        vm().heap.verifyCanGC();
    10141014    if (isRope())

  • trunk/Source/JavaScriptCore/runtime/OptionsList.h

    r262523 r262570  
    154154    v(Bool, useProbeOSRExit, false, Normal, nullptr) \
    155155    v(Bool, printEachOSRExit, false, Normal, nullptr) \
     156    v(Bool, validateDoesGC, ASSERT_ENABLED, Normal, nullptr) \
    156157    v(Bool, validateGraph, false, Normal, nullptr) \
    157158    v(Bool, validateGraphAtEachPhase, false, Normal, nullptr) \

  • trunk/Source/JavaScriptCore/runtime/RegExpMatchesArray.h

    r262513 r262570  
    11/*
    2  *  Copyright (C) 2008-2019 Apple Inc. All Rights Reserved.
     2 *  Copyright (C) 2008-2020 Apple Inc. All Rights Reserved.
    33 *
    44 *  This library is free software; you can redistribute it and/or
    … …  
    6464    RegExp* regExp, unsigned startOffset, MatchResult& result)
    6565{
    66     if (validateDFGDoesGC)
     66    if constexpr (validateDFGDoesGC)
    6767        vm.heap.verifyCanGC();
    6868
Note: See TracChangeset for help on using the changeset viewer.