Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[git-webkit] Link git output capture to logging level #48

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JonWBedard
Copy link
Member

d6b428f

[git-webkit] Link git output capture to logging level
https://bugs.webkit.org/show_bug.cgi?id=233637

Reviewed by NOBODY (OOPS!).

WIP

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.git):

https://bugs.webkit.org/show_bug.cgi?id=233637

Reviewed by NOBODY (OOPS!).

WIP

* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/setup.py:
(Setup.git):
@aj062
Copy link
Member

aj062 commented Feb 14, 2022

rs=me

@JonWBedard JonWBedard added merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue labels Mar 1, 2022
@JonWBedard JonWBedard added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Mar 14, 2022
@JonWBedard JonWBedard added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Nightly Build labels Mar 29, 2022
webkit-commit-queue pushed a commit that referenced this pull request May 13, 2022
…arget labels.

https://bugs.webkit.org/show_bug.cgi?id=240370

Reviewed by Saam Barati.

Disassemblies used to look like this:

     0x10e480ff8:    ldurb    w17, [x0, #7]
     0x10e480ffc:    cmp      w17, #0
     0x10e481000:    b.hi   0x10e48103c
     0x10e481004:    stur     x0, [fp, #-72]
     ...
     0x10e481040:    movk     x3, #0xfffe, lsl #48
     0x10e481044:    b        0x10e4814f4
     0x10e481048:    nop

With this patch, it will now look like this:

       <748> 0x10e120aec:    ldurb    w17, [x0, #7]
       <752> 0x10e120af0:    cmp      w17, #0
       <756> 0x10e120af4:    b.hi     0x10e120b30 -> <816>
       <760> 0x10e120af8:    stur     x0, [fp, #-80]
       ...
       <820> 0x10e120b34:    movk     x3, #0xfffe, lsl #48
       <824> 0x10e120b38:    b        0x10e120fc8 -> <1992>
       <828> 0x10e120b3c:    nop

1. Each instruction pc is now prefixed with a pc index i.e. the offset of the
   pc address from the start of the compilation unit e.g. <756>.

2. Relative branches now show the branch target as a pc index (effectively, an
   internal label in this compilation unit) in addition to the pc address e.g.
   the "-> <816>" in:
       <756> 0x10e120af4:    b.hi     0x10e120b30 -> <816>

   Also fixed a formatting bug where the space between relative branch instructions
   and their target pc was short 2 spaces.

3. If the relative branch target is a known thunk, the disassembler will now
   print the thunk label e.g.

       <828> 0x10e12033c:    bl       0x10e0f0a00 -> <thunk: get_from_scope thunk>
      <1476> 0x10e120dc4:    cbnz     x16, 0x10e104100 -> <thunk: handleExceptionWithCallFrameRollback>
      <2368> 0x10e121140:    b        0x10e10c000 -> <thunk: DFG OSR exit generation thunk>

   Introduced a FINALIZE_THUNK macro that will be used instead of FINALIZE_CODE in
   thunk generators.  By doing so, thunk labels will automatically be registered
   with the disassembler, and will be used for the above look up.

   Thunk label registration is only done if disassembly is enabled.

4. If the branch target is neither an internal label nor a thunk, then the
   disassembler will print some useful info about it to the best of its
   knowledge e.g.

       <168> 0x10e1002e8:    b        0x10e120b60 -> <JIT PC>
       <168> 0x10e1002e8:    b        0x10e120b60 -> <LLInt PC>
       <168> 0x10e1002e8:    b        0x10e120b60 -> <unknown>

5. The disassemble() function now takes 2 additional arguments: codeStart, and
   codeEnd.  These are needed so that the disassembler can compute the pc index
   for each instruction, as well as determine if a branch target is internal to
   this compilation unit, or pointing out of it.

This feature is currently only supported for the ARM64 disassembler.

Printing of JIT operation labels (via movz + movk + indirect branch) is not yet
supported.

* assembler/LinkBuffer.cpp:
(JSC::LinkBuffer::finalizeCodeWithDisassemblyImpl):
* assembler/LinkBuffer.h:
(JSC::LinkBuffer::setIsThunk):
* b3/air/AirDisassembler.cpp:
(JSC::B3::Air::Disassembler::dump):
* dfg/DFGDisassembler.cpp:
(JSC::DFG::Disassembler::dumpDisassembly):
* dfg/DFGThunks.cpp:
(JSC::DFG::osrExitGenerationThunkGenerator):
(JSC::DFG::osrEntryThunkGenerator):
* disassembler/ARM64/A64DOpcode.cpp:
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset):
(JSC::ARM64Disassembler::A64DOpcodeConditionalBranchImmediate::format):
* disassembler/ARM64/A64DOpcode.h:
(JSC::ARM64Disassembler::A64DOpcode::A64DOpcode):
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset): Deleted.
* disassembler/ARM64Disassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/CapstoneDisassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/Disassembler.cpp:
(JSC::disassemble):
(JSC::disassembleAsynchronously):
(JSC::ensureThunkLabelMap):
(JSC::registerThunkLabel):
(JSC::labelForThunk):
* disassembler/Disassembler.h:
(JSC::tryToDisassemble):
* disassembler/RISCV64Disassembler.cpp:
(JSC::tryToDisassemble):
* disassembler/X86Disassembler.cpp:
(JSC::tryToDisassemble):
* ftl/FTLThunks.cpp:
(JSC::FTL::genericGenerationThunkGenerator):
(JSC::FTL::slowPathCallThunkGenerator):
* jit/JIT.cpp:
(JSC::JIT::consistencyCheckGenerator):
* jit/JITCall.cpp:
(JSC::JIT::returnFromBaselineGenerator):
* jit/JITDisassembler.cpp:
(JSC::JITDisassembler::dump):
(JSC::JITDisassembler::dumpDisassembly):
* jit/JITDisassembler.h:
* jit/JITOpcodes.cpp:
(JSC::JIT::valueIsFalseyGenerator):
(JSC::JIT::valueIsTruthyGenerator):
(JSC::JIT::op_throw_handlerGenerator):
(JSC::JIT::op_enter_handlerGenerator):
(JSC::JIT::op_check_traps_handlerGenerator):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::slow_op_get_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_private_name_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_private_name_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_del_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_del_by_val_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_get_by_id_with_this_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::slow_op_put_by_id_callSlowOperationThenCheckExceptionGenerator):
(JSC::JIT::generateOpResolveScopeThunk):
(JSC::JIT::slow_op_resolve_scopeGenerator):
(JSC::JIT::generateOpGetFromScopeThunk):
(JSC::JIT::slow_op_get_from_scopeGenerator):
(JSC::JIT::slow_op_put_to_scopeGenerator):
* jit/SlowPathCall.cpp:
(JSC::JITSlowPathCall::generateThunk):
* jit/SpecializedThunkJIT.h:
(JSC::SpecializedThunkJIT::finalize):
* jit/ThunkGenerator.h:
* jit/ThunkGenerators.cpp:
(JSC::handleExceptionGenerator):
(JSC::handleExceptionWithCallFrameRollbackGenerator):
(JSC::popThunkStackPreservesAndHandleExceptionGenerator):
(JSC::checkExceptionGenerator):
(JSC::throwExceptionFromCallSlowPathGenerator):
(JSC::linkCallThunkGenerator):
(JSC::linkPolymorphicCallThunkGenerator):
(JSC::virtualThunkFor):
(JSC::nativeForGenerator):
(JSC::arityFixupGenerator):
(JSC::unreachableGenerator):
(JSC::stringGetByValGenerator):
(JSC::boundFunctionCallGenerator):
(JSC::remoteFunctionCallGenerator):
* llint/LLIntThunks.cpp:
(JSC::LLInt::generateThunkWithJumpTo):
(JSC::LLInt::generateThunkWithJumpToPrologue):
(JSC::LLInt::generateThunkWithJumpToLLIntReturnPoint):
(JSC::LLInt::createJSGateThunk):
(JSC::LLInt::createWasmGateThunk):
(JSC::LLInt::createTailCallGate):
(JSC::LLInt::tagGateThunk):
(JSC::LLInt::untagGateThunk):
* yarr/YarrDisassembler.cpp:
(JSC::Yarr::YarrDisassembler::dump):
(JSC::Yarr::YarrDisassembler::dumpDisassembly):
* yarr/YarrDisassembler.h:

Canonical link: https://commits.webkit.org/250547@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294180 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request May 17, 2022
…n constants.

https://bugs.webkit.org/show_bug.cgi?id=240443

Reviewed by Yusuke Suzuki.

Source/JavaScriptCore:

1. Added a Options::needDisassemblySupport() option.  This option is true if
   any of the JSC disassembly options are enabled.  This allows us to trivially
   check if we need to enable additional infrastructure (like those added in this
   patch) to support disassembly.

2. Refactor the Disassembler's thunkLabelMap into a more generic labelMap.  It
   can now map a pointer to a CString or a const char* label.

3. Enable JITOperationList infrastructure when ENABLE(JIT_OPERATION_DISASSEMBLY)
   is true.  This adds a name to the JITOperationAnnotation record.  Since this is
   guarded by ENABLE(JIT_OPERATION_DISASSEMBLY), we can trivially turn this part
   off later if adding this name adds too much size burden (or if any unexpected
   issues arise).  Turning this off will only disable disassembly of JIT operation
   names.  Disassembly of other constants (see below) will still be supported.

   If Options::needDisassemblySupport() is true, the JITOperationList will
   register all JIT operations (and the LLInt ones as well) with the Disassembler
   labelMap.

4. Removed the < > brackets around branch target labels except for internal pc
   index labels (e.g. <716>) and <unknown> (when we don't know what the branch
   target is).  The < > notation takes up space and doesn't add any info.  The
   branch targets now look like this:

      <32> 0x1180102c0:    b.ne     0x1180101c0 -> thunk: native Tail With Saved Tags Call trampoline
      <92> 0x11801c87c:    b        0x118008980 -> thunk: LLInt function for call jump to prologue thunk
    <3508> 0x1198e16b4:    b        0x1198bc1a0 -> JIT PC

   Internal pc index labels will still use the < > brackets:

    <3476> 0x1198e1694:    b.eq     0x1198e16a0 -> <3488>

5. Added ARM64 disassembler support to compute the value of a constant being
   loaded into a register using "MoveWide" instructions e.g. movz and movk.

   When the disassembler sees a movz or movn or mov (of the MoveWide variety),
   the disassembler initializes a m_builtConstant value.  With each subsequent
   movk instruction with the same destination register, the disassembler splices
   in the corresponding 16-bit immediate.

   After disassembling a MoveWide instruction, it will check if the next
   instruction is also a MoveWide instruction targeting the same destination
   register.  If so, the constant is still being build: hence, it does nothing and
   continues.

   If the next instruction is (a) a MoveWide instruction targeting a different
   register, (b) not a MoveWide instruction, or (c) we've reached the end of the
   LinkBuffer (i.e. there's no next instruction), then the constant building for
   the current target register is complete.

6. Added ARM64 disassembler support for looking up constants (built in (5) above)
   and printing symbolic info / names of those constants.

   With ENABLE(JIT_OPERATION_DISASSEMBLY), we now have JIT operation names e.g.

     <176> 0x118010270:    movk     x3, #0x9f6e, lsl #48 -> 0x102dc8950 operationVMHandleException
     <164> 0x1180180a4:    movk     x8, #0xe5c, lsl #48 -> 0x102db9420 operationVirtualCall

   Apart from looking up the constant in the Disassembler labelMap, it also looks
   up some commonly used constants e.g.

   a. VM pointers:

     <156> 0x11801105c:    movk     x0, #0x1, lsl #32 -> 0x139435000 vm

   b. Some VM internal pointers (more can be added later as needed):

      <24> 0x118014d18:    movk     x17, #0x1, lsl #32 -> 0x13943ee78 vm +40568: vm.topCallFrame
      <76> 0x118014d4c:    movk     x17, #0x1, lsl #32 -> 0x139441a10 vm +51728: vm.m_exception
     <196> 0x118011244:    movk     x17, #0x1, lsl #32 -> 0x1394417d0 vm +51152: vm.targetMachinePCForThrow
     <208> 0x1198e09d0:    movk     x17, #0x1, lsl #32 -> 0x104432cc0 vm +52416: vm.m_doesGC

   c. VM related pointers (only 1 for now; not VM fields, but hangs off of VM):

      <12> 0x11801938c:    movk     x1, #0x1, lsl #32 -> 0x1052cd3c8 vm scratchBuffer.m_buffer

   d. Well known PtrTags:

     <204> 0x11801124c:    movz     x16, #0x6f9b -> 0x6f9b ExceptionHandlerPtrTag ?
     <212> 0x1180150d4:    movz     x16, #0x593 -> 0x593 JSEntryPtrTag ?
     <168> 0x1180183a8:    movz     lr, #0xb389 -> 0xb389 OperationPtrTag ?

     * the ? is because we cannot be certain that the 16-bit constant is a PtrTag.
       It may just be a coincidence that the value is the same.  However, the user
       can trivially look at the surrounding disassembled code, and be able to
       tell if the value is used as a PtrTag.

   For constants that are not found in the known sets:

   e. Small 16-bit constant (prints decimal value for convenience):

     <200> 0x1198e09c8:    movz     x17, #0x2cc0 -> 11456

   f. Unknown pointers that aren't in the JIT and LLINT regions will simply print
      the pointer / constant:

      <88> 0x1198e0958:    movz     x17, #0x2088
      <92> 0x1198e095c:    movk     x17, #0x30d, lsl #16
      <96> 0x1198e0960:    movk     x17, #0x1, lsl #32 -> 0x1030d2088

      <unknown> is only used in relative branches.

7. Enhanced the Integrity::isSanePointer() check to reject pointer values that
   are less than 4G on CPU(ADDRESS64) targets for OS(DARWIN).  No sane pointer
   should be in the lowest 4G address region.

   This also helps the disassembler more quickly filter out constant values that
   aren't pointers.

   This change affects DFG's speculationFromCell(), which is the only place that
   may affect user facing performance.  However, I think the impact should be
   insignificant (since the added check is cheap).

* assembler/JITOperationList.cpp:
(JSC::JITOperationList::populatePointersInJavaScriptCore):
(JSC::llintOperations):
(JSC::JITOperationList::populatePointersInJavaScriptCoreForLLInt):
(JSC::JITOperationList::addDisassemblyLabels):
(JSC::JITOperationList::populateDisassemblyLabelsInJavaScriptCore):
(JSC::JITOperationList::populateDisassemblyLabelsInJavaScriptCoreForLLInt):
(JSC::JITOperationList::populateDisassemblyLabelsInEmbedder):
* assembler/JITOperationList.h:
(JSC::JITOperationList::populatePointersInJavaScriptCore):
(JSC::JITOperationList::populatePointersInJavaScriptCoreForLLInt):
* assembler/JITOperationValidation.h:
* assembler/LinkBuffer.cpp:
(JSC::LinkBuffer::finalizeCodeWithDisassemblyImpl):
* b3/testb3_1.cpp:
(main):
* disassembler/ARM64/A64DOpcode.cpp:
(JSC::ARM64Disassembler::A64DOpcode::appendPCRelativeOffset):
(JSC::ARM64Disassembler::MoveWideFormatTrait::rejectedResult):
(JSC::ARM64Disassembler::MoveWideFormatTrait::acceptedResult):
(JSC::ARM64Disassembler::MoveWideIsValidTrait::rejectedResult):
(JSC::ARM64Disassembler::MoveWideIsValidTrait::acceptedResult):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::handlePotentialDataPointer):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::handlePotentialPtrTag):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::parse):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::format):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::isValid):
* disassembler/ARM64/A64DOpcode.h:
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::baseFormat):
(JSC::ARM64Disassembler::A64DOpcodeMoveWide::formatBuffer):
* disassembler/Disassembler.cpp:
(JSC::Disassembler::ensureLabelMap):
(JSC::registerLabel):
(JSC::labelFor):
(JSC::ensureThunkLabelMap): Deleted.
(JSC::registerThunkLabel): Deleted.
(JSC::labelForThunk): Deleted.
* disassembler/Disassembler.h:
* jsc.cpp:
(jscmain):
* runtime/Gate.h:
* runtime/JSCPtrTag.cpp:
(JSC::ptrTagName):
* runtime/JSCPtrTag.h:
* runtime/Options.cpp:
(JSC::Options::recomputeDependentOptions):
* runtime/OptionsList.h:
* runtime/VM.cpp:
(JSC::VM::isScratchBuffer):
* runtime/VM.h:
* tools/Integrity.h:
(JSC::Integrity::isSanePointer):

Source/WebCore:

* bindings/js/WebCoreJITOperations.cpp:
(WebCore::populateJITOperations):
(WebCore::populateDisassemblyLabels):
* bindings/js/WebCoreJITOperations.h:
(WebCore::populateDisassemblyLabels):
(WebCore::populateJITOperations):
* testing/js/WebCoreTestSupport.cpp:
(WebCoreTestSupport::populateJITOperations):
(WebCoreTestSupport::populateDisassemblyLabels):
* testing/js/WebCoreTestSupport.h:
(WebCoreTestSupport::populateDisassemblyLabels):
(WebCoreTestSupport::populateJITOperations):

Source/WTF:

1. Added a ENABLE(JIT_OPERATION_DISASSEMBLY) flag.
   Currently, this feature relies on an USE(APPLE_INTERNAL_SDK) for enumerating
   JIT operations similar to ENABLE(JIT_OPERATION_VALIDATION).  It is also
   restricted to ENABLE(DISASSEMBLER) && CPU(ARM64E).

* wtf/PlatformCallingConventions.h:
* wtf/PlatformEnable.h:

Canonical link: https://commits.webkit.org/250631@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294287 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-commit-queue pushed a commit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241856

Reviewed by Yusuke Suzuki.

1. Ruby treats numeric 0 as truthy.  However, there's a test in arm64LowerMalformedLoadStoreAddresses
   which assumes a value of 0 would be false.  As a result, we see offlineasm emit inefficient LLInt
   code like this:
    ".loc 3 821\n"        "movz x16, #0 \n"                    // LowLevelInterpreter64.asm:821
                          "add x13, x3, x16 \n"
                          "ldr x0, [x13] \n"

  ...  instead of this:
    ".loc 3 821\n"        "ldr x0, [x3] \n"                    // LowLevelInterpreter64.asm:821

   This patch fixes this.

2. offlineasm's emitARM64MoveImmediate chooses to use `movn` instead of `movz` based on whether a
   64-bit value is negative or not.  Instead, it should be making that decision based on the number of
   halfwords (16-bits) in the value that is 0xffff vs 0.  As a result, offlineasm emits code like this:
    ".loc 1 1638\n"       "movn x27, #1, lsl #48 \n"           // LowLevelInterpreter.asm:1638
                          "movk x27, #0, lsl #32 \n"
                          "movk x27, #0, lsl #16 \n"
                          "movk x27, #0 \n"

  ...  instead of this:
    ".loc 1 1638\n"       "movz x27, #65534, lsl #48 \n"       // LowLevelInterpreter.asm:1638

   This patch fixes this.

3. offlineasm is trivially assuming the range of immediate offsets for ldr/str instructions is
   [-255..4095].  However, that's only the range for byte sized load-stores.  For 32-bit, the range
   is actually [-255..16380].  For 64-bit, the range is actually [-255..32760].  As a result,
    offlineasm emits code like this:
    ".loc 1 633\n"        "movn x16, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x16 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "movz x16, #16088 \n"                // LowLevelInterpreter.asm:1519
                          "add x17, x3, x16 \n"
                          "ldr x3, [x17] \n"

  ...  instead of this:
    ".loc 1 633\n"        "movn x17, #16383 \n"                // LowLevelInterpreter.asm:633
    ".loc 1 1518\n"       "and x3, x3, x17 \n"                 // LowLevelInterpreter.asm:1518
    ".loc 1 1519\n"       "ldr x3, [x3, #16088] \n"            // LowLevelInterpreter.asm:1519

   This patch fixes this for 64-bit and 32-bit load-stores.  16-bit load-stores also has a wider
   range, but for now, it will continue to use the conservative range.

   This patch also introduces an `isMalformedArm64LoadAStoreAddress` so that this range check can be
   done consistently in all the places that checks for it.

4. offlineasm is eagerly emitting no-op arguments in instructions, e.g. "lsl #0", and adding 0.
   As a result, offlineasm emits code like this:
    ".loc 3 220\n"        "movz x13, #51168, lsl #0 \n"        // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13, lsl #0 \n"
                          "ldr w4, [x17, #0] \n"

  ...  instead of this:
    ".loc 3 220\n"        "movz x13, #51168 \n"                // LowLevelInterpreter64.asm:220
                          "add x17, x1, x13 \n"
                          "ldr w4, [x17] \n"

   This unnecessary arguments are actually very common throughout the emitted LLIntAssembly.h.

   This patch removes these unnecessary arguments, which makes the emitted LLInt code more human
   readable due to less clutter.

This patch has passed the testapi and JSC stress tests with a Release build on an M1 Mac.

I also manually verified that the emitARM64MoveImmediate code is working properly by
hacking up LowLevelInterpreter64.asm to emit moves of constants of different values in
the ranges, and for load-store instructions of different sizes, and visually inspecting
the emitted code.

* Source/JavaScriptCore/offlineasm/arm64.rb:

Canonical link: https://commits.webkit.org/251771@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295766 268f45cc-cd09-0410-ab3c-d52691b4dbfc
webkit-early-warning-system pushed a commit to MenloDorian/Webkit that referenced this pull request Jun 23, 2022
https://bugs.webkit.org/show_bug.cgi?id=241905

Reviewed by Yusuke Suzuki.

offlineasm used to emit this LLInt code:
    ".loc 1 996\n"        "ldr x19, [x0] \n"                   // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldr x20, [x0, WebKit#8] \n"               // LowLevelInterpreter.asm:997
    ".loc 1 998\n"        "ldr x21, [x0, WebKit#16] \n"              // LowLevelInterpreter.asm:998
    ".loc 1 999\n"        "ldr x22, [x0, WebKit#24] \n"              // LowLevelInterpreter.asm:999
    ...
    ".loc 1 1006\n"       "ldr d8, [x0, WebKit#80] \n"               // LowLevelInterpreter.asm:1006
    ".loc 1 1007\n"       "ldr d9, [x0, WebKit#88] \n"               // LowLevelInterpreter.asm:1007
    ".loc 1 1008\n"       "ldr d10, [x0, WebKit#96] \n"              // LowLevelInterpreter.asm:1008
    ".loc 1 1009\n"       "ldr d11, [x0, WebKit#104] \n"             // LowLevelInterpreter.asm:1009
    ...

Now, it can emit this instead:
    ".loc 1 996\n"        "ldp x19, x20, [x0, #0] \n"          // LowLevelInterpreter.asm:996
    ".loc 1 997\n"        "ldp x21, x22, [x0, WebKit#16] \n"         // LowLevelInterpreter.asm:997
    ...
    ".loc 1 1001\n"       "ldp d8, d9, [x0, WebKit#80] \n"           // LowLevelInterpreter.asm:1001
    ".loc 1 1002\n"       "ldp d10, d11, [x0, WebKit#96] \n"         // LowLevelInterpreter.asm:1002
    ...

Also, there was some code that kept recomputing the base address of a sequence of load/store
instructions.  For example,
    ".loc 6 902\n"        "add x13, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:902
                          "ldr x0, [x13, WebKit#48] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x1, [x13, WebKit#56] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x2, [x13, WebKit#64] \n"
                          "add x13, sp, x10, lsl WebKit#3 \n"
                          "ldr x3, [x13, WebKit#72] \n"
    ...

For such places, we observe that the base address is the same for every load/store instruction
in the sequence, and precompute it in the LLInt asm code to help out the offline asm.  This
allows the offlineasm to now emit this more efficient code instead:
    ".loc 6 896\n"        "add x10, sp, x10, lsl WebKit#3 \n"        // WebAssembly.asm:896
    ".loc 6 898\n"        "ldp x0, x1, [x10, WebKit#48] \n"          // WebAssembly.asm:898
                          "ldp x2, x3, [x10, WebKit#64] \n"
    ...

* Source/JavaScriptCore/llint/LowLevelInterpreter.asm:
* Source/JavaScriptCore/llint/WebAssembly.asm:
* Source/JavaScriptCore/offlineasm/arm64.rb:
* Source/JavaScriptCore/offlineasm/instructions.rb:

Canonical link: https://commits.webkit.org/251799@main
Constellation added a commit to Constellation/WebKit that referenced this pull request Aug 25, 2024
https://bugs.webkit.org/show_bug.cgi?id=278617
rdar://134636872

Reviewed by NOBODY (OOPS!).

When the next op is also one character PatternCharacterClass, backtracking should not load index register
since the next op will override it anyway.

Before:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
                <4472> 0x10f3a2078:    ldur     x1, [sp, WebKit#64]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
                <4476> 0x10f3a207c:    ldur     x1, [sp, WebKit#48]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
                <4480> 0x10f3a2080:    ldur     x1, [sp, WebKit#32]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
                <4484> 0x10f3a2084:    ldur     x1, [sp, WebKit#16]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <4488> 0x10f3a2088:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <4492> 0x10f3a208c:    b        0x10f3a1e78 -> <3960>

After:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <3656> 0x11e840e48:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <3660> 0x11e840e4c:    b        0x11e840ca8 -> <3240>

* Source/JavaScriptCore/yarr/YarrJIT.cpp:
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Aug 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=278617
rdar://134636872

Reviewed by Yijia Huang.

When the next op is also one character PatternCharacterClass, backtracking should not load index register
since the next op will override it anyway.

Before:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
                <4472> 0x10f3a2078:    ldur     x1, [sp, WebKit#64]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
                <4476> 0x10f3a207c:    ldur     x1, [sp, WebKit#48]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
                <4480> 0x10f3a2080:    ldur     x1, [sp, WebKit#32]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
                <4484> 0x10f3a2084:    ldur     x1, [sp, WebKit#16]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <4488> 0x10f3a2088:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <4492> 0x10f3a208c:    b        0x10f3a1e78 -> <3960>

After:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <3656> 0x11e840e48:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <3660> 0x11e840e4c:    b        0x11e840ca8 -> <3240>

* Source/JavaScriptCore/yarr/YarrJIT.cpp:

Canonical link: https://commits.webkit.org/282745@main
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Aug 26, 2024
https://bugs.webkit.org/show_bug.cgi?id=278617
rdar://134636872

Reviewed by Yijia Huang.

When the next op is also one character PatternCharacterClass, backtracking should not load index register
since the next op will override it anyway.

Before:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
                <4472> 0x10f3a2078:    ldur     x1, [sp, WebKit#64]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
                <4476> 0x10f3a207c:    ldur     x1, [sp, WebKit#48]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
                <4480> 0x10f3a2080:    ldur     x1, [sp, WebKit#32]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
                <4484> 0x10f3a2084:    ldur     x1, [sp, WebKit#16]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <4488> 0x10f3a2088:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <4492> 0x10f3a208c:    b        0x10f3a1e78 -> <3960>

After:
      57:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a2,0x1f0b2,0x1f0c2,0x1f0d2)]
      56:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a3,0x1f0b3,0x1f0c3,0x1f0d3)]
      55:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a4,0x1f0b4,0x1f0c4,0x1f0d4)]
      54:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a5,0x1f0b5,0x1f0c5,0x1f0d5)]
      53:Term PatternCharacterClass checked-offset:(10) [Unicode:(0x1f0a6,0x1f0b6,0x1f0c6,0x1f0d6)]
                <3656> 0x11e840e48:    ldur     x1, [sp]
      52:BodyAlternativeNext minimum-size:(10),checked-offset:(10)
                <3660> 0x11e840e4c:    b        0x11e840ca8 -> <3240>

* Source/JavaScriptCore/yarr/YarrJIT.cpp:

Canonical link: https://commits.webkit.org/282746@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants