[regexp] Move mode to RegExpMacroAssembler [v8/v8 : main]

[Patrick Thier (Gerrit)]

Attention needed from Jakob Linke

Patrick Thier voted and added 1 comment

Votes added by Patrick Thier

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Patrick Thier . resolved

PTAL

Open in Gerrit

Related details

Attention is currently required from:

• Jakob Linke

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 2

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Jakob Linke <jgr...@chromium.org>

Gerrit-Comment-Date: Tue, 18 Nov 2025 10:36:59 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Jakob Linke (Gerrit)]

Attention needed from Patrick Thier

Jakob Linke voted and added 1 comment

Votes added by Jakob Linke

Code-Review

+1

1 comment

File src/regexp/regexp-macro-assembler.h

Line 49, Patchset 2 (Latest): enum Mode { LATIN1 = 1, UC16 = 2 };

Jakob Linke . unresolved

Suggestion: add asserts that the values match char byte sizes.

Open in Gerrit

Related details

Attention is currently required from:

• Patrick Thier

Submit Requirements:

• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 2

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Patrick Thier <pth...@chromium.org>

Gerrit-Comment-Date: Tue, 18 Nov 2025 10:43:48 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Patrick Thier (Gerrit)]

Attention needed from Jakob Linke

Patrick Thier added 2 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Patrick Thier . resolved

PTAL again due to additional, unrequested change that made sense to me based on your comment.

File src/regexp/regexp-macro-assembler.h

Line 49, Patchset 2: enum Mode { LATIN1 = 1, UC16 = 2 };

Jakob Linke . unresolved

Suggestion: add asserts that the values match char byte sizes.

Patrick Thier

I also moved `char_size()` out of the specific sub-classes to `RegExpMacroAssembler` now and added the asserts there.

Open in Gerrit

Related details

Attention is currently required from:

• Jakob Linke

Submit Requirements:

• Code-Owners
• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 5

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Jakob Linke <jgr...@chromium.org>

Gerrit-Comment-Date: Tue, 18 Nov 2025 12:27:53 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Jakob Linke <jgr...@chromium.org>

[Jakob Linke (Gerrit)]

Attention needed from Patrick Thier

Jakob Linke voted and added 1 comment

Votes added by Jakob Linke

Code-Review

+1

1 comment

File src/regexp/regexp-macro-assembler.h

Line 49, Patchset 2: enum Mode { LATIN1 = 1, UC16 = 2 };

Jakob Linke . resolved

Suggestion: add asserts that the values match char byte sizes.

Patrick Thier

I also moved `char_size()` out of the specific sub-classes to `RegExpMacroAssembler` now and added the asserts there.

Jakob Linke

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:

• Patrick Thier

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 5

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Patrick Thier <pth...@chromium.org>

Gerrit-Comment-Date: Tue, 18 Nov 2025 12:34:58 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Patrick Thier <pth...@chromium.org>

Comment-In-Reply-To: Jakob Linke <jgr...@chromium.org>

[Patrick Thier (Gerrit)]

Patrick Thier voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 5

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-CC: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Comment-Date: Tue, 18 Nov 2025 12:36:21 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[V8 LUCI CQ (Gerrit)]

V8 LUCI CQ submitted the change

Change information

Commit message:

[regexp] Move mode to RegExpMacroAssembler

... instead of having it in each sub-class individually.
Previously it was only used in sub-classes of
NativeRegExpMacroAssembler, but we also want to use it in the
Bytecode-Generator.

Drive-by: Also move char_size() from individual sub-classes to
RegExpMacroAssembler.

Bug: 437003349

Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7164997

Commit-Queue: Patrick Thier <pth...@chromium.org>

Reviewed-by: Jakob Linke <jgr...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#103785}

Files:

• M src/regexp/arm/regexp-macro-assembler-arm.cc
• M src/regexp/arm/regexp-macro-assembler-arm.h
• M src/regexp/arm64/regexp-macro-assembler-arm64.cc
• M src/regexp/arm64/regexp-macro-assembler-arm64.h
• M src/regexp/ia32/regexp-macro-assembler-ia32.cc
• M src/regexp/ia32/regexp-macro-assembler-ia32.h
• M src/regexp/loong64/regexp-macro-assembler-loong64.cc
• M src/regexp/loong64/regexp-macro-assembler-loong64.h
• M src/regexp/mips64/regexp-macro-assembler-mips64.cc
• M src/regexp/mips64/regexp-macro-assembler-mips64.h
• M src/regexp/ppc/regexp-macro-assembler-ppc.cc
• M src/regexp/ppc/regexp-macro-assembler-ppc.h
• M src/regexp/regexp-bytecode-generator.cc
• M src/regexp/regexp-bytecode-generator.h
• M src/regexp/regexp-macro-assembler.cc
• M src/regexp/regexp-macro-assembler.h
• M src/regexp/regexp.cc
• M src/regexp/riscv/regexp-macro-assembler-riscv.cc
• M src/regexp/riscv/regexp-macro-assembler-riscv.h
• M src/regexp/s390/regexp-macro-assembler-s390.cc
• M src/regexp/s390/regexp-macro-assembler-s390.h
• M src/regexp/x64/regexp-macro-assembler-x64.cc
• M src/regexp/x64/regexp-macro-assembler-x64.h
• M test/unittests/regexp/regexp-unittest.cc

Change size: L

Delta: 24 files changed, 234 insertions(+), 272 deletions(-)

Branch: refs/heads/main

Submit Requirements:

• Code-Review: +1 by Jakob Linke

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: merged

Gerrit-Project: v8/v8

Gerrit-Branch: main

Gerrit-Change-Id: I89b20419d718892b477f8a9643cc105aef401b29

Gerrit-Change-Number: 7164997

Gerrit-PatchSet: 6

Gerrit-Owner: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: Jakob Linke <jgr...@chromium.org>

Gerrit-Reviewer: Patrick Thier <pth...@chromium.org>

Gerrit-Reviewer: V8 LUCI CQ <v8-s...@luci-project-accounts.iam.gserviceaccount.com>