Changeset 262523 in webkit


Ignore:
Timestamp:
Jun 3, 2020 4:18:30 PM (4 years ago)
Author:
Tadeu Zagallo
Message:

Disable B3 hoistLoopInvariantValues by default
​https://bugs.webkit.org/show_bug.cgi?id=212511
<rdar://problem/63813245>

Reviewed by Mark Lam.

JSTests:

  • microbenchmarks/interpreter-wasm.js: Added.

(key.in.Module.Module.hasOwnProperty):
(quit_):
(locateFile):
(ENVIRONMENT_IS_NODE.read_):
(ENVIRONMENT_IS_NODE.readBinary):
(ENVIRONMENT_IS_NODE.quit_):
(ENVIRONMENT_IS_NODE.Module.string_appeared_here):
(else.read_):
(else.readBinary):
(else.quit_):
(else):
(else.xhr.onload):
(else.readAsync):
(else.setWindowTitle):
(key.in.moduleOverrides.moduleOverrides.hasOwnProperty):
(dynamicAlloc):
(getNativeTypeSize):
(warnOnce):
(convertJsFunctionToWasm):
(addFunctionWasm):
(removeFunctionWasm):
(setTempRet0):
(setValue):
(assert):
(getCFunc):
(toC.string_appeared_here):
(convertReturnValue):
(ccall):
(UTF8ArrayToString):
(UTF8ToString):
(stringToUTF8Array):
(stringToUTF8):
(lengthBytesUTF8):
(allocateUTF8OnStack):
(writeArrayToMemory):
(writeAsciiToMemory):
(updateGlobalBufferAndViews):
(callRuntimeCallbacks):
(preRun):
(initRuntime):
(preMain):
(exitRuntime):
(postRun):
(addOnPreRun):
(addOnPostRun):
(addRunDependency):
(removeRunDependency):
(hasPrefix):
(isDataURI):
(isFileURI):
(getBinary):
(getBinaryPromise):
(createWasm.receiveInstance):
(createWasm.receiveInstantiatedSource):
(createWasm.instantiateArrayBuffer):
(createWasm.instantiateAsync.):
(createWasm.instantiateAsync):
(createWasm):
(ATINIT.push.func):
(demangle):
(demangleAll):
(_emscripten_get_sbrk_ptr):
(_emscripten_memcpy_big):
(abortOnCannotGrowMemory):
(_emscripten_resize_heap):
(PATH.splitPath):
(PATH.normalizeArray):
(PATH.normalize):
(PATH.dirname):
(PATH.basename):
(PATH.extname):
(PATH.join):
(PATH.join2):
(SYSCALLS.printChar):
(SYSCALLS.getStr):
(SYSCALLS.get64):
(_fd_write):
(_setTempRet0):
(_wasm_call_ctors.Module.string_appeared_here):
(_main.Module.string_appeared_here):
(_malloc.Module.string_appeared_here):
(_errno_location.Module.string_appeared_here):
(_free.Module.string_appeared_here):
(stackSave.Module.string_appeared_here):
(stackAlloc.Module.string_appeared_here):
(stackRestore.Module.string_appeared_here):
(growWasmMemory.Module.string_appeared_here):
(dynCall_ii.Module.string_appeared_here):
(dynCall_iiii.Module.string_appeared_here):
(dynCall_jiji.Module.string_appeared_here):
(ExitStatus):
(dependenciesFulfilled):
(callMain):
(run.doRun):
(run):
(exit):

  • microbenchmarks/interpreter-wasm.wasm: Added.

Source/JavaScriptCore:

The hoistLoopInvariantValues optimization in B3 does not calculate the cost of hoisting the candidates.
For example, in the test case provided with the bug, a switch inside a loop can lead to hoisting the body
of several switch cases which would never be executed. Other than leading to worse runtime, this also
increases the pressure in the register allocate, leading to worse compile times (~10x worse in this case).
I have added a FIXME to consider adding cost calculation and re-enabling this pass, but given that we
already have LICM in DFG, it should be ok to disable it for now.

  • b3/B3Generate.cpp:

(JSC::B3::generateToAir):

  • runtime/OptionsList.h:

Tools:

Enable the B3 hoistLoopInvariantValues pass in one of our existing configurations to
avoid bit rot since we'd like to re-enable it eventually.

  • Scripts/run-jsc-stress-tests:
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r262425 r262523  
     12020-06-03  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Disable B3 hoistLoopInvariantValues by default
     4        https://bugs.webkit.org/show_bug.cgi?id=212511
     5        <rdar://problem/63813245>
     6
     7        Reviewed by Mark Lam.
     8
     9        * microbenchmarks/interpreter-wasm.js: Added.
     10        (key.in.Module.Module.hasOwnProperty):
     11        (quit_):
     12        (locateFile):
     13        (ENVIRONMENT_IS_NODE.read_):
     14        (ENVIRONMENT_IS_NODE.readBinary):
     15        (ENVIRONMENT_IS_NODE.quit_):
     16        (ENVIRONMENT_IS_NODE.Module.string_appeared_here):
     17        (else.read_):
     18        (else.readBinary):
     19        (else.quit_):
     20        (else):
     21        (else.xhr.onload):
     22        (else.readAsync):
     23        (else.setWindowTitle):
     24        (key.in.moduleOverrides.moduleOverrides.hasOwnProperty):
     25        (dynamicAlloc):
     26        (getNativeTypeSize):
     27        (warnOnce):
     28        (convertJsFunctionToWasm):
     29        (addFunctionWasm):
     30        (removeFunctionWasm):
     31        (setTempRet0):
     32        (setValue):
     33        (assert):
     34        (getCFunc):
     35        (toC.string_appeared_here):
     36        (convertReturnValue):
     37        (ccall):
     38        (UTF8ArrayToString):
     39        (UTF8ToString):
     40        (stringToUTF8Array):
     41        (stringToUTF8):
     42        (lengthBytesUTF8):
     43        (allocateUTF8OnStack):
     44        (writeArrayToMemory):
     45        (writeAsciiToMemory):
     46        (updateGlobalBufferAndViews):
     47        (callRuntimeCallbacks):
     48        (preRun):
     49        (initRuntime):
     50        (preMain):
     51        (exitRuntime):
     52        (postRun):
     53        (addOnPreRun):
     54        (addOnPostRun):
     55        (addRunDependency):
     56        (removeRunDependency):
     57        (hasPrefix):
     58        (isDataURI):
     59        (isFileURI):
     60        (getBinary):
     61        (getBinaryPromise):
     62        (createWasm.receiveInstance):
     63        (createWasm.receiveInstantiatedSource):
     64        (createWasm.instantiateArrayBuffer):
     65        (createWasm.instantiateAsync.):
     66        (createWasm.instantiateAsync):
     67        (createWasm):
     68        (__ATINIT__.push.func):
     69        (demangle):
     70        (demangleAll):
     71        (_emscripten_get_sbrk_ptr):
     72        (_emscripten_memcpy_big):
     73        (abortOnCannotGrowMemory):
     74        (_emscripten_resize_heap):
     75        (PATH.splitPath):
     76        (PATH.normalizeArray):
     77        (PATH.normalize):
     78        (PATH.dirname):
     79        (PATH.basename):
     80        (PATH.extname):
     81        (PATH.join):
     82        (PATH.join2):
     83        (SYSCALLS.printChar):
     84        (SYSCALLS.getStr):
     85        (SYSCALLS.get64):
     86        (_fd_write):
     87        (_setTempRet0):
     88        (___wasm_call_ctors.Module.string_appeared_here):
     89        (_main.Module.string_appeared_here):
     90        (_malloc.Module.string_appeared_here):
     91        (___errno_location.Module.string_appeared_here):
     92        (_free.Module.string_appeared_here):
     93        (stackSave.Module.string_appeared_here):
     94        (stackAlloc.Module.string_appeared_here):
     95        (stackRestore.Module.string_appeared_here):
     96        (__growWasmMemory.Module.string_appeared_here):
     97        (dynCall_ii.Module.string_appeared_here):
     98        (dynCall_iiii.Module.string_appeared_here):
     99        (dynCall_jiji.Module.string_appeared_here):
     100        (ExitStatus):
     101        (dependenciesFulfilled):
     102        (callMain):
     103        (run.doRun):
     104        (run):
     105        (exit):
     106        * microbenchmarks/interpreter-wasm.wasm: Added.
     107
    11082020-06-02  Saam Barati  <sbarati@apple.com>
    2109

  • trunk/Source/JavaScriptCore/ChangeLog

    r262517 r262523  
     12020-06-03  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Disable B3 hoistLoopInvariantValues by default
     4        https://bugs.webkit.org/show_bug.cgi?id=212511
     5        <rdar://problem/63813245>
     6
     7        Reviewed by Mark Lam.
     8
     9        The hoistLoopInvariantValues optimization in B3 does not calculate the cost of hoisting the candidates.
     10        For example, in the test case provided with the bug, a switch inside a loop can lead to hoisting the body
     11        of several switch cases which would never be executed. Other than leading to worse runtime, this also
     12        increases the pressure in the register allocate, leading to worse compile times (~10x worse in this case).
     13        I have added a FIXME to consider adding cost calculation and re-enabling this pass, but given that we
     14        already have LICM in DFG, it should be ok to disable it for now.
     15
     16        * b3/B3Generate.cpp:
     17        (JSC::B3::generateToAir):
     18        * runtime/OptionsList.h:
     19
    1202020-06-03  Mark Lam  <mark.lam@apple.com>
    221

  • trunk/Source/JavaScriptCore/b3/B3Generate.cpp

    r261755 r262523  
    8484        reduceDoubleToFloat(procedure);
    8585        reduceStrength(procedure);
    86         hoistLoopInvariantValues(procedure);
     86        // FIXME: Re-enable B3 hoistLoopInvariantValues
     87        // https://bugs.webkit.org/show_bug.cgi?id=212651
     88        if (Options::useB3HoistLoopInvariantValues())
     89            hoistLoopInvariantValues(procedure);
    8790        if (eliminateCommonSubexpressions(procedure))
    8891            eliminateCommonSubexpressions(procedure);

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

    r262014 r262523  
    422422    v(Unsigned, maxB3TailDupBlockSize, 3, Normal, nullptr) \
    423423    v(Unsigned, maxB3TailDupBlockSuccessors, 3, Normal, nullptr) \
     424    v(Bool, useB3HoistLoopInvariantValues, false, Normal, nullptr) \
    424425    \
    425426    v(Bool, useDollarVM, false, Restricted, "installs the $vm debugging tool in global objects") \

  • trunk/Tools/ChangeLog

    r262515 r262523  
     12020-06-03  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Disable B3 hoistLoopInvariantValues by default
     4        https://bugs.webkit.org/show_bug.cgi?id=212511
     5        <rdar://problem/63813245>
     6
     7        Reviewed by Mark Lam.
     8
     9        Enable the B3 hoistLoopInvariantValues pass in one of our existing configurations to
     10        avoid bit rot since we'd like to re-enable it eventually.
     11
     12        * Scripts/run-jsc-stress-tests:
     13
    1142020-06-03  Wenson Hsieh  <wenson_hsieh@apple.com>
    215

  • trunk/Tools/Scripts/run-jsc-stress-tests

    r262229 r262523  
    755755
    756756def runFTLNoCJITNoInlineValidate(*optionalTestSpecificOptions)
    757     run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", "--airForceBriggsAllocator=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + optionalTestSpecificOptions))
     757    run("ftl-no-cjit-no-inline-validate", "--validateGraph=true", "--maximumInliningDepth=1", "--airForceBriggsAllocator=true", "--useB3HoistLoopInvariantValues=true", *(FTL_OPTIONS + NO_CJIT_OPTIONS + optionalTestSpecificOptions))
    758758end
    759759
Note: See TracChangeset for help on using the changeset viewer.