Skip to content

[git-webkit] Add land command#16

Closed
JonWBedard wants to merge 1 commit into
WebKit:mainfrom
JonWBedard:eng/git-webkit-land
Closed

[git-webkit] Add land command#16
JonWBedard wants to merge 1 commit into
WebKit:mainfrom
JonWBedard:eng/git-webkit-land

Conversation

@JonWBedard

@JonWBedard JonWBedard commented Oct 15, 2021

Copy link
Copy Markdown
Member

2c52e40

[git-webkit] Add land command
https://bugs.webkit.org/show_bug.cgi?id=231821
<rdar://problem/84309339 >

Reviewed by NOBODY (OOPS!).

* Tools/Scripts/git-webkit: WebKit still uses Svn as it's canonical source of truth.
* Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/local/git.py:
(Git._to_git_ref): Convert identifiers or revisions to git refs.
(Git.checkout): Use shared _to_git_ref function.
(Git.rebase): Support rebasing identifiers.
(Git.pull): Generalize computation of the gmtoffset.
(Git.diff_lines): Output diff between two refs.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/local/git.py:
(Git.__init__): Add branch, push and diff commands.
(Git.filter_branch): Support sed.
(Git.pull): Mock `git pull`
(Git.move_branch): Mock `git rebase`.
(Git.push): Mock `git push`.
(Git.reset): Mock `git reset HEAD~#`.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/__init__.py:
(main): Add land command.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/land.py: Added.
(Land.parser):
(Land.main): Rebase the current branch against it's root, assign the reviewer, check for 'OOPS' messages,
rebase against target branch, update target branch ref, optionally canonicalize, push (or svn dcommit) and
update and close pull-request with information on the landed commit.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/scm_base.py:
(ScmBase.gmtoffset): Add shared computation of GMT offset.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/git_unittest.py:
(TestGit.test_commits): `git log` should exclude base ref.
(TestGit.test_commits_branch): Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/land_unittest.py: Added.
(repository):
(TestLand):
(TestLandGitHub):
(TestLandBitBucket):

@JonWBedard
JonWBedard marked this pull request as draft October 18, 2021 19:01
@JonWBedard JonWBedard changed the title [git-webkit] Add land command (Part 1) [git-webkit] Add land command Oct 19, 2021
xanlpz referenced this pull request in xanlpz/WebKit Oct 26, 2021
https://bugs.webkit.org/show_bug.cgi?id=232303

Reviewed by NOBODY (OOPS!).

This patch does two things:

Add the .size and .type directives to every llint "function"
(global, llint opcode, 'glue'). This allows a debugger to tell you
in what logical function you are inside the giant chunk of code
that is the llint interpreter. So instead of something like this:

(gdb) x/5i $pc
  => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>:  b.n     0xf5f8af6c <wasmLLIntPCRangeStart+3868>
     0xf5f8af62 <wasmLLIntPCRangeStart+3858>:  ldr     r2, [r7, #8]
     0xf5f8af64 <wasmLLIntPCRangeStart+3860>:  ldr     r2, [r2, WebKit#28]
     0xf5f8af66 <wasmLLIntPCRangeStart+3862>:  subs    r0, #16
     0xf5f8af68 <wasmLLIntPCRangeStart+3864>:  ldr.w   r0, [r2, r0, lsl #3]

you get something like this:

(gdb) x/5i $pc
  => 0xf5f8c770 <wasm_f32_add+12>:      bge.n   0xf5f8c77c <wasm_f32_add+24>
     0xf5f8c772 <wasm_f32_add+14>:      add.w   r6, r7, r9, lsl #3
     0xf5f8c776 <wasm_f32_add+18>:      vldr    d0, [r6]
     0xf5f8c77a <wasm_f32_add+22>:      b.n     0xf5f8c78c <wasm_f32_add+40>
     0xf5f8c77c <wasm_f32_add+24>:      ldr     r2, [r7, #8]

The other change adds a local symbol (in addition to an internal
label) to all the "glue" labels. That allows wasm opcodes to be
seen by the debugger (and the user to break on them), among other
things.

* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
* offlineasm/asm.rb: emit the .size and .type directives for every
llint "function" on ELF systems.
webkit-commit-queue pushed a commit that referenced this pull request Oct 26, 2021
https://bugs.webkit.org/show_bug.cgi?id=232303

Patch by Xan López <[email protected]> on 2021-10-26
Reviewed by Mark Lam.

This patch does two things:

Add the .size and .type directives to every llint "function"
(global, llint opcode, 'glue'). This allows a debugger to tell you
in what logical function you are inside the giant chunk of code
that is the llint interpreter. So instead of something like this:

(gdb) x/5i $pc
  => 0xf5f8af60 <wasmLLIntPCRangeStart+3856>:  b.n     0xf5f8af6c <wasmLLIntPCRangeStart+3868>
     0xf5f8af62 <wasmLLIntPCRangeStart+3858>:  ldr     r2, [r7, #8]
     0xf5f8af64 <wasmLLIntPCRangeStart+3860>:  ldr     r2, [r2, #28]
     0xf5f8af66 <wasmLLIntPCRangeStart+3862>:  subs    r0, #16
     0xf5f8af68 <wasmLLIntPCRangeStart+3864>:  ldr.w   r0, [r2, r0, lsl #3]

you get something like this:

(gdb) x/5i $pc
  => 0xf5f8c770 <wasm_f32_add+12>:      bge.n   0xf5f8c77c <wasm_f32_add+24>
     0xf5f8c772 <wasm_f32_add+14>:      add.w   r6, r7, r9, lsl #3
     0xf5f8c776 <wasm_f32_add+18>:      vldr    d0, [r6]
     0xf5f8c77a <wasm_f32_add+22>:      b.n     0xf5f8c78c <wasm_f32_add+40>
     0xf5f8c77c <wasm_f32_add+24>:      ldr     r2, [r7, #8]

The other change adds a local symbol (in addition to an internal
label) to all the "glue" labels. That allows wasm opcodes to be
seen by the debugger (and the user to break on them), among other
things.

* CMakeLists.txt: tell offlineasm we use the ELF binary format on Linux.
* llint/LowLevelInterpreter.cpp: emit a non-local label for "glue" labels.
* offlineasm/asm.rb: emit the .size and .type directives for every
llint "function" on ELF systems.

Canonical link: https://commits.webkit.org/243545@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284868 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@JonWBedard JonWBedard self-assigned this Nov 3, 2021
@JonWBedard
JonWBedard marked this pull request as ready for review November 3, 2021 19:17
@JonWBedard
JonWBedard force-pushed the eng/git-webkit-land branch 3 times, most recently from 9df9b77 to f217ebb Compare November 4, 2021 00:02

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: do we expect space between ** and kwargs?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys.version_info >= (3, 6)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: no space between ** and kwargs(?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sys.version_info >= (3, 6) ?

@JonWBedard

Copy link
Copy Markdown
Member Author

For reference, #32 was landed with this script.

https://bugs.webkit.org/show_bug.cgi?id=231821
<rdar://problem/84309339>

Reviewed by NOBODY (OOPS!).

* Tools/Scripts/git-webkit: WebKit still uses Svn as it's canonical source of truth.
* Tools/Scripts/libraries/webkitscmpy/setup.py: Bump version.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/__init__.py: Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/local/git.py:
(Git._to_git_ref): Convert identifiers or revisions to git refs.
(Git.checkout): Use shared _to_git_ref function.
(Git.rebase): Support rebasing identifiers.
(Git.pull): Generalize computation of the gmtoffset.
(Git.diff_lines): Output diff between two refs.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/local/git.py:
(Git.__init__): Add branch, push and diff commands.
(Git.filter_branch): Support sed.
(Git.pull): Mock `git pull`
(Git.move_branch): Mock `git rebase`.
(Git.push): Mock `git push`.
(Git.reset): Mock `git reset HEAD~#`.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/__init__.py:
(main): Add land command.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/land.py: Added.
(Land.parser):
(Land.main): Rebase the current branch against it's root, assign the reviewer, check for 'OOPS' messages,
rebase against target branch, update target branch ref, optionally canonicalize, push (or svn dcommit) and
update and close pull-request with information on the landed commit.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/scm_base.py:
(ScmBase.gmtoffset): Add shared computation of GMT offset.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/git_unittest.py:
(TestGit.test_commits): `git log` should exclude base ref.
(TestGit.test_commits_branch): Ditto.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/land_unittest.py: Added.
(repository):
(TestLand):
(TestLandGitHub):
(TestLandBitBucket):
@JonWBedard

Copy link
Copy Markdown
Member Author

@JonWBedard JonWBedard closed this Nov 4, 2021
@JonWBedard
JonWBedard deleted the eng/git-webkit-land branch November 4, 2021 18:50
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
webkit-early-warning-system pushed a commit to Constellation/WebKit that referenced this pull request Dec 22, 2022
https://bugs.webkit.org/show_bug.cgi?id=249765
rdar://103631099

Reviewed by Mark Lam.

In ARM64, we are leveraging LDR style address, which can take 32bit index in addressing and zero-extend / sign-extend that in load/store.
This is useful since WasmAddress' index is 32bit and we need to zero-extend it. However, we cannot use this addressing when there is an
offset since this addressing cannot encode offset. As a result, we are emitting Move32 and Add64 when there is an offset.
However, ARM64 can do even better for that case since ARM64 add / sub instructions also support LDR style extension.

This patch adds AddZeroExtend64 and AddSignExtend64. They take 32bit second operand and extend it before adding. This is particularly useful
when computing WasmAddress. We also leverage this in AirIRGenerator.

In the added testb3, the generated code is changed as follows.

    Before:
        O2: testWasmAddressWithOffset()...
        Generated JIT code for Compilation:
            Code at [0x115f74980, 0x115f749a0):
                     <0> 0x115f74980:    pacibsp
                     <4> 0x115f74984:    stp      fp, lr, [sp, #-16]!
                     <8> 0x115f74988:    mov      fp, sp
                    <12> 0x115f7498c:    ubfx     x0, x0, #0, WebKit#32; emitSave
                    <16> 0x115f74990:    add      x0, x2, x0
                    <20> 0x115f74994:    sturb    w1, [x0, WebKit#1]
                    <24> 0x115f74998:    ldp      fp, lr, [sp], WebKit#16
                    <28> 0x115f7499c:    retab

    After:
        O2: testWasmAddressWithOffset()...
        Generated JIT code for Compilation:
            Code at [0x121108980, 0x1211089a0):
                     <0> 0x121108980:    pacibsp
                     <4> 0x121108984:    stp      fp, lr, [sp, #-16]!
                     <8> 0x121108988:    mov      fp, sp
                    <12> 0x12110898c:    add      x0, x2, w0, uxtw; emitSave
                    <16> 0x121108990:    sturb    w1, [x0, WebKit#1]
                    <20> 0x121108994:    ldp      fp, lr, [sp], WebKit#16
                    <24> 0x121108998:    retab

* Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::addZeroExtend64):
(JSC::MacroAssemblerARM64::addSignExtend64):
* Source/JavaScriptCore/b3/B3LowerToAir.cpp:
* Source/JavaScriptCore/b3/air/AirInstInlines.h:
(JSC::B3::Air::isAddZeroExtend64Valid):
(JSC::B3::Air::isAddSignExtend64Valid):
* Source/JavaScriptCore/b3/air/AirOpcode.opcodes:

Canonical link: https://commits.webkit.org/258259@main
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Jan 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=251063
rdar://104585575

Reviewed by Mark Lam and Justin Michaud.

This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace.
The example is as follows.

    frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt]
    frame WebKit#1: 0x000000010ec44204 0x10eccc5dc
    frame WebKit#2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined)
    frame WebKit#3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40)
    frame WebKit#4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40)
    frame WebKit#5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3)
    frame WebKit#6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0)
    frame WebKit#7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0)
    frame WebKit#8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#10: 0x000000010ec3c728
    frame WebKit#11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt]
    frame WebKit#12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt]
    frame WebKit#13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt]
    frame WebKit#14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt]
    frame WebKit#15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt]
    frame WebKit#16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt]
    ...

* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::dump const):

Canonical link: https://commits.webkit.org/259262@main
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Mar 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=254128
rdar://106906971

Reviewed by Mark Lam.

This patch adds better disassembler support to new WasmBBQJIT (since previous old BBQ had good disassembler. So now it was missing).
It dumps code with Wasm Opcode information as follows.

    ...
               <128> 0x10d000080:    b.pl     0x10d00044c -> <1100>
    [     0x3] Block
    [     0x5] Block
    [     0x7] GetLocal
               <132> 0x10d000084:    ldur     w0, [fp, #-12]
    [     0x9] I32Const
    [     0xb] I32Add
               <136> 0x10d000088:    add      w0, w0, WebKit#16
    [     0xc] TeeLocal
               <140> 0x10d00008c:    stur     w0, [fp, #-20]
               <144> 0x10d000090:    ldur     w0, [fp, #-20]
    [     0xe] I32Load
               <148> 0x10d000094:    ldr      w0, [x22, w0, uxtw]
    [    0x11] TeeLocal
               <152> 0x10d000098:    stur     w0, [fp, #-16]
               <156> 0x10d00009c:    ldur     w0, [fp, #-16]
    [    0x13] BrIf
               <160> 0x10d0000a0:    mov      x8, x0
               <164> 0x10d0000a4:    cbz      w8, 0x10d0000ac -> <172>
               <168> 0x10d0000a8:    b        0x10d0000e4 -> <228>
    ...

* Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:
* Source/JavaScriptCore/Sources.txt:
* Source/JavaScriptCore/wasm/WasmB3IRGenerator.h:
* Source/JavaScriptCore/wasm/WasmBBQDisassembler.cpp: Added.
(JSC::Wasm::BBQDisassembler::dump):
(JSC::Wasm::BBQDisassembler::dumpHeader):
(JSC::Wasm::BBQDisassembler::dumpVectorForInstructions):
(JSC::Wasm::BBQDisassembler::dumpForInstructions):
(JSC::Wasm::BBQDisassembler::dumpDisassembly):
* Source/JavaScriptCore/wasm/WasmBBQDisassembler.h: Added.
(JSC::Wasm::BBQDisassembler::setStartOfCode):
(JSC::Wasm::BBQDisassembler::setOpcode):
(JSC::Wasm::BBQDisassembler::setEndOfOpcode):
(JSC::Wasm::BBQDisassembler::setEndOfCode):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::BBQJIT):
(JSC::Wasm::BBQJIT::endTopLevel):
(JSC::Wasm::BBQJIT::willParseOpcode):
(JSC::Wasm::BBQJIT::finalize):
(JSC::Wasm::BBQJIT::takeDisassembler):
(JSC::Wasm::parseAndCompileBBQ):
* Source/JavaScriptCore/wasm/WasmBBQPlan.cpp:
(JSC::Wasm::BBQPlan::dumpDisassembly):

Canonical link: https://commits.webkit.org/261835@main
CGQAQ pushed a commit to CGQAQ/WebKit that referenced this pull request Aug 29, 2023
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
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 21, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102
rdar://145222010

Reviewed by Yusuke Suzuki.

Added the notion of a string list to a parsed RegExp that is in the form of
  /^(?:break|case|which|do|for)/ with an optional trailing $.
Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern.

This change involves recognizing beginning of string anchored alternations of strings while parsing and
then treating the generation of JIT code differently for these patterns.  This includes changing how
conditional branching works, specifically that instead of the "fall through on match" for each term,
to a "jump on match" for the whole alternation.

The current code generated for the "case" elternative is:
   8:Term PatternCharacter checked-offset:(3) 'c'
               <156> 0x11381430c:    add      w1, w1, WebKit#2
               <160> 0x113814310:    cmp      w1, w2
               <164> 0x113814314:    b.hi     0x113814444 -> <468>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <168> 0x113814318:    sub      x17, x0, WebKit#4
               <172> 0x11381431c:    ldr      w17, [x17, x1]
               <176> 0x113814320:    movz     w16, #0x6163
               <180> 0x113814324:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <184> 0x113814328:    cmp      w17, w16
               <188> 0x11381432c:    b.ne     0x113814444 -> <468>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:NestedAlternativeNext minimum-size:(5),checked-offset:(5)
               <192> 0x113814330:    movz     x16, #0x4444
               <196> 0x113814334:    movk     x16, #0x1381, lsl WebKit#16
               <200> 0x113814338:    movk     x16, #0x8001, lsl WebKit#32
               <204> 0x11381433c:    movk     x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC
               <208> 0x113814340:    stur     x16, [sp, WebKit#8]
               <212> 0x113814344:    b        0x113814404 -> <404>
With some additional backtracking code:
   9:NestedAlternativeNext minimum-size:(4),checked-offset:(4)
               <468> 0x113814444:    sub      w1, w1, WebKit#2
               <472> 0x113814448:    b        0x113814348 -> <216>

With this change, the processing of "case" becomes:
   9:StringListAlternativeNext minimum-size:(4),checked-offset:(4)
               <132> 0x12a8285c4:    sub      w1, w1, WebKit#1
               <136> 0x12a8285c8:    cmp      w1, w2
               <140> 0x12a8285cc:    b.hi     0x12a8285e8 -> <168>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <144> 0x12a8285d0:    sub      x17, x0, WebKit#4
               <148> 0x12a8285d4:    ldr      w17, [x17, x1]
               <152> 0x12a8285d8:    movz     w16, #0x6163
               <156> 0x12a8285dc:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <160> 0x12a8285e0:    cmp      w17, w16
               <164> 0x12a8285e4:    b.eq     0x12a82866c -> <300>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:StringListAlternativeNext minimum-size:(5),checked-offset:(5)
With no backtracking code.

We are able to eliminate one branch and the saving of the continuation PC for backtracking.
The code size to process these string list RegExp is reduces.  For the example RegExp above,
the prior version created 1940 bytes (485 instructions) of code while the code created with this
1392 bytes (345 instructions) of code, a nearly 30% reduction in code.

This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark:

                                 Baseline               YarrStringList

regexp-keyword-parsing      136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

<geometric>                 136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

* JSTests/microbenchmarks/regexp-keyword-parsing.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
(let.re.break.case.catch.continue.debugger.default.else.finally.if):
(let.re1.break.case.catch.continue.debugger.default.else.finally.if):
* JSTests/stress/regexp-parsing-tokens.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
* Source/JavaScriptCore/yarr/YarrJIT.cpp:
* Source/JavaScriptCore/yarr/YarrPattern.cpp:
(JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):
(JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses):
(JSC::Yarr::PatternAlternative::dump):
(JSC::Yarr::PatternTerm::dump):
* Source/JavaScriptCore/yarr/YarrPattern.h:
(JSC::Yarr::PatternTerm::PatternTerm):
(JSC::Yarr::PatternAlternative::PatternAlternative):

Canonical link: https://commits.webkit.org/290791@main
webkit-commit-queue pushed a commit to msaboff/WebKit that referenced this pull request Feb 24, 2025
https://bugs.webkit.org/show_bug.cgi?id=288102
rdar://145222010

Reviewed by Yusuke Suzuki.

Added the notion of a string list to a parsed RegExp that is in the form of
  /^(?:break|case|which|do|for)/ with an optional trailing $.
Such a RegExp will not backtrack and therefore we can streamline the code we emit for such a pattern.

This change involves recognizing beginning of string anchored alternations of strings while parsing and
then treating the generation of JIT code differently for these patterns.  This includes changing how
conditional branching works, specifically that instead of the "fall through on match" for each term,
to a "jump on match" for the whole alternation.

Fixed a bug in the original version where we weren't properly checking the nested alternatives to see
if they only contain fixed single count PatternCharacter terms.

The current code generated for the "case" elternative is:
   8:Term PatternCharacter checked-offset:(3) 'c'
               <156> 0x11381430c:    add      w1, w1, WebKit#2
               <160> 0x113814310:    cmp      w1, w2
               <164> 0x113814314:    b.hi     0x113814444 -> <468>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <168> 0x113814318:    sub      x17, x0, WebKit#4
               <172> 0x11381431c:    ldr      w17, [x17, x1]
               <176> 0x113814320:    movz     w16, #0x6163
               <180> 0x113814324:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <184> 0x113814328:    cmp      w17, w16
               <188> 0x11381432c:    b.ne     0x113814444 -> <468>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:NestedAlternativeNext minimum-size:(5),checked-offset:(5)
               <192> 0x113814330:    movz     x16, #0x4444
               <196> 0x113814334:    movk     x16, #0x1381, lsl WebKit#16
               <200> 0x113814338:    movk     x16, #0x8001, lsl WebKit#32
               <204> 0x11381433c:    movk     x16, #0xc973, lsl WebKit#48 -> 0x113814444 JIT PC
               <208> 0x113814340:    stur     x16, [sp, WebKit#8]
               <212> 0x113814344:    b        0x113814404 -> <404>
With some additional backtracking code:
   9:NestedAlternativeNext minimum-size:(4),checked-offset:(4)
               <468> 0x113814444:    sub      w1, w1, WebKit#2
               <472> 0x113814448:    b        0x113814348 -> <216>

With this change, the processing of "case" becomes:
   9:StringListAlternativeNext minimum-size:(4),checked-offset:(4)
               <132> 0x12a8285c4:    sub      w1, w1, WebKit#1
               <136> 0x12a8285c8:    cmp      w1, w2
               <140> 0x12a8285cc:    b.hi     0x12a8285e8 -> <168>
  10:Term PatternCharacter checked-offset:(4) 'c'
               <144> 0x12a8285d0:    sub      x17, x0, WebKit#4
               <148> 0x12a8285d4:    ldr      w17, [x17, x1]
               <152> 0x12a8285d8:    movz     w16, #0x6163
               <156> 0x12a8285dc:    movk     w16, #0x6573, lsl WebKit#16 -> 0x65736163
               <160> 0x12a8285e0:    cmp      w17, w16
               <164> 0x12a8285e4:    b.eq     0x12a82866c -> <300>
  11:Term PatternCharacter checked-offset:(4) 'a' already handled
  12:Term PatternCharacter checked-offset:(4) 's' already handled
  13:Term PatternCharacter checked-offset:(4) 'e' already handled
  14:StringListAlternativeNext minimum-size:(5),checked-offset:(5)
With no backtracking code.

We are able to eliminate one branch and the saving of the continuation PC for backtracking.
The code size to process these string list RegExp is reduces.  For the example RegExp above,
the prior version created 1940 bytes (485 instructions) of code while the code created with this
1392 bytes (345 instructions) of code, a nearly 30% reduction in code.

This change is a ~18% progression on the new regexp-keyword-parsing microbenchmark:

                                 Baseline               YarrStringList

regexp-keyword-parsing      136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

<geometric>                 136.7065+-0.9807     ^    116.0161+-1.1791        ^ definitely 1.1783x faster

* JSTests/microbenchmarks/regexp-keyword-parsing.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
(let.re.break.case.catch.continue.debugger.default.else.finally.if):
(let.re1.break.case.catch.continue.debugger.default.else.finally.if):
* JSTests/stress/regexp-parsing-tokens.js: Added.
(arrayToString):
(objectToString):
(dumpValue):
(compareArray):
(compareGroups):
(testRegExp):
(testRegExpSyntaxError):
* Source/JavaScriptCore/yarr/YarrJIT.cpp:
* Source/JavaScriptCore/yarr/YarrPattern.cpp:
(JSC::Yarr::YarrPatternConstructor::atomParenthesesEnd):
(JSC::Yarr::YarrPatternConstructor::checkForTerminalParentheses):
(JSC::Yarr::PatternAlternative::dump):
(JSC::Yarr::PatternTerm::dump):
* Source/JavaScriptCore/yarr/YarrPattern.h:
(JSC::Yarr::PatternTerm::PatternTerm):
(JSC::Yarr::PatternAlternative::PatternAlternative):

Canonical link: https://commits.webkit.org/290982@main
Wowfunhappy referenced this pull request in Wowfunhappy/WebKit Jun 18, 2026
The backport installed a global NSEvent monitor (+[WKView installEventMonitorOnce]) that intercepted mouse/scroll/key events and dispatched them to WKView, on the belief that layer-tree mirror subviews intercepted hit-testing. That was a MISDIAGNOSIS: WKView hosts the WebContent layer tree as a plain CALayer sublayer of its backing layer (MinimalPageClient enterAcceleratedCompositingMode/setRemoteLayerTreeRootNode, [[m_view layer] addSublayer:]) with ZERO NSView subviews. CALayers don't participate in NSView -hitTest:, so nothing ever intercepted events. Worse, the monitor consumed events before -[NSWindow sendEvent:] (where AppKit activates the window on click), creating the very click-to-activate problem it then re-patched.

Remove the monitor and rely on AppKit's natural responder chain; WKView's own scrollWheel:/keyDown:/mouseDown:/mouseMoved: handlers receive events directly. Keep the -hitTest: override as upstream-equivalent insurance (upstream WebViewImpl::hitTest does the same redirect).

Verified live: main-view links/scroll/typing/right-click; hover (#17) fires :hover on a fresh page AND with the address bar focused; Reader scroll (#39) works via natural routing to the topmost ReaderWKView; and every secondary WKView that relied on the monitor -- uBlock popover (#14/#23), Web Inspector (#16), App Store (#72), Top Sites (#13), Preferences>Extensions (#24) -- stays interactive. No crashes. Retires the #62 event-monitor keystone and supersedes #39's monitor routing. (The DDG-logo black box, a separate #56 IOSurface-backing symptom, is root-caused but not fixed here.)

Co-Authored-By: Claude Opus 4.8 <[email protected]>
webkit-commit-queue pushed a commit that referenced this pull request Jul 23, 2026
…s' per-flex-item static helpers

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

Reviewed by Antti Koivisto.

The per-item static helpers in FlexFormattingUtils took the flex container as an
explicit (const RenderFlexibleBox&, const RenderBox& flexItem) pair, even though a
flex item's parent is always the flex container. Drop the container argument from
these twelve statics and derive it internally from the item's parent, mirroring
what ScopedCrossAxisOverrideForFlexItem already does:

    auto& flexBox = downcast<RenderFlexibleBox>(*flexItem.parent());

The two that only forwarded the container to other statics (useContentBasedMinimumSize,
flexBasisForFlexItem) no longer need it at all. All callers drop the container argument:
the utils' own static-to-static calls and instance forwarders, RenderFlexibleBox's proxies
and ScopedCrossAxisOverrideForFlexItem, and LayoutIntegration::FlexLayout (its positioned
flex items are direct out-of-flow children, so their parent is the container too). The
container-only statics (isColumnFlow, isHorizontalFlow, computeGap, ...) still take it.

Also correct the cross-axis step comments in FlexFormattingContext::layout(): they
labelled a section "9.6 (#13 - #16)" although the block also runs the §9.4 steps (#9,
dependency around the container's used-cross-size finalization (#15), not by spec number,
so the comment now says so.

No change in behavior.

* Source/WebCore/layout/formattingContexts/flex/FlexFormattingContext.cpp:
(WebCore::FlexFormattingContext::layout):
* Source/WebCore/layout/formattingContexts/flex/FlexFormattingUtils.cpp:
(WebCore::FlexFormattingUtils::crossAxisMarginExtentForFlexItem):
(WebCore::FlexFormattingUtils::preferredMainSizeLengthForFlexItem):
(WebCore::FlexFormattingUtils::minMainSizeLengthForFlexItem):
(WebCore::FlexFormattingUtils::preferredCrossSizeLengthForFlexItem):
(WebCore::FlexFormattingUtils::mainAxisOverflowForFlexItem):
(WebCore::FlexFormattingUtils::hasAutoMarginsInCrossAxis):
(WebCore::FlexFormattingUtils::useContentBasedMinimumSize):
(WebCore::FlexFormattingUtils::preferredAspectRatioForFlexItem const):
(WebCore::FlexFormattingUtils::needToStretchFlexItemLogicalHeight const):
(WebCore::FlexFormattingUtils::innerCrossSizeForFlexItem):
(WebCore::FlexFormattingUtils::availableAlignmentSpaceForFlexItem const):
(WebCore::FlexFormattingUtils::marginBoxAscentForFlexItem const):
(WebCore::FlexFormattingUtils::mainAxisIsFlexItemInlineAxis):
(WebCore::FlexFormattingUtils::flexBasisForFlexItem):
(WebCore::FlexFormattingUtils::alignmentForFlexItem):
(WebCore::FlexFormattingUtils::hasDefiniteCrossSizeForFlexItem):
(WebCore::FlexFormattingUtils::crossAxisMarginExtentForFlexItem const):
(WebCore::FlexFormattingUtils::preferredMainSizeLengthForFlexItem const):
(WebCore::FlexFormattingUtils::minMainSizeLengthForFlexItem const):
(WebCore::FlexFormattingUtils::preferredCrossSizeLengthForFlexItem const):
(WebCore::FlexFormattingUtils::hasAutoMarginsInCrossAxis const):
(WebCore::FlexFormattingUtils::useContentBasedMinimumSize const):
(WebCore::FlexFormattingUtils::innerCrossSizeForFlexItem const):
(WebCore::FlexFormattingUtils::mainAxisIsFlexItemInlineAxis const):
(WebCore::FlexFormattingUtils::flexBasisForFlexItem const):
(WebCore::FlexFormattingUtils::alignmentForFlexItem const):
(WebCore::FlexFormattingUtils::hasDefiniteCrossSizeForFlexItem const):
* Source/WebCore/layout/formattingContexts/flex/FlexFormattingUtils.h:
* Source/WebCore/layout/integration/flex/LayoutIntegrationFlexLayout.cpp:
(WebCore::LayoutIntegration::FlexLayout::firstLineBaseline const):
(WebCore::LayoutIntegration::FlexLayout::lastLineBaseline const):
(WebCore::LayoutIntegration::FlexLayout::baselineFlexItemInLine const):
(WebCore::LayoutIntegration::FlexLayout::staticCrossAxisPositionForPositionedFlexItem):
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::ScopedCrossAxisOverrideForFlexItem::ScopedCrossAxisOverrideForFlexItem):
(WebCore::RenderFlexibleBox::canUseFlexItemForPercentageResolution):
(WebCore::RenderFlexibleBox::computeBlockAxisContentSizeForFlexItem):
(WebCore::RenderFlexibleBox::setOverridingMainSizeForFlexItem):
(WebCore::RenderFlexibleBox::resetAutoMarginsAndLogicalTopInCrossAxis):
(WebCore::RenderFlexibleBox::useContentBasedMinimumBlockSize const):
(WebCore::RenderFlexibleBox::hasStretchedFlexItemWithAspectRatio const):
(WebCore::RenderFlexibleBox::mainAxisIsFlexItemInlineAxis const):
(WebCore::RenderFlexibleBox::flexBasisForFlexItem const):
(WebCore::RenderFlexibleBox::alignmentForFlexItem const):
(WebCore::RenderFlexibleBox::hasDefiniteCrossSizeForFlexItem const):
(WebCore::RenderFlexibleBox::flexItemMainSizeIsDefinite):

Canonical link: https://commits.webkit.org/317776@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants