[maglev] Refactor TryWithArrayIterationArgs into layered helpers [v8/v8 : main]

[Olivier Flückiger (Gerrit)]

Attention needed from Victor Gomes

Olivier Flückiger voted and added 1 comment

Votes added by Olivier Flückiger

Auto-Submit	+1
Commit-Queue	+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Olivier Flückiger . resolved

ptal

Open in Gerrit

Related details

Attention is currently required from:

• Victor Gomes

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: Iade87e9f73c9d0ded8be6957dd7f36e8eef460d5

Gerrit-Change-Number: 8139484

Gerrit-PatchSet: 4

Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>

Gerrit-Attention: Victor Gomes <victo...@chromium.org>

Gerrit-Comment-Date: Fri, 24 Jul 2026 11:48:46 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Victor Gomes (Gerrit)]

Attention needed from Olivier Flückiger

Victor Gomes voted and added 4 comments

Votes added by Victor Gomes

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Victor Gomes . unresolved

Nice! LGTM % forcing ReduceResult

File src/maglev/maglev-reducer-inl.h

Line 1132, Patchset 4 (Latest): ValueNode* length) -> MaybeReduceResult {

Victor Gomes . unresolved

ReduceResult

Line 940, Patchset 4 (Latest): ValueNode* length) -> MaybeReduceResult {

Victor Gomes . unresolved

ReduceResult

Line 930, Patchset 4 (Latest): return Reducer(elements_kind, elements, length);

Victor Gomes . unresolved

Let's restrict Reducer to return a ReduceResult, not a MaybeReduceResult. Because, by this point we have already inserted map checks and emitted loads, we should not fail the reduction.

We can either do:
```
ReduceResult result = Reducer(elements_kind, elements, length);
return result;
```

or we can change the typename ReducerCb to base::FunctionRef<ReduceResult(...args...)

Up to you.

Open in Gerrit

Related details

Attention is currently required from:

• Olivier Flückiger

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: Iade87e9f73c9d0ded8be6957dd7f36e8eef460d5

Gerrit-Change-Number: 8139484

Gerrit-PatchSet: 4

Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>

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

Gerrit-Attention: Olivier Flückiger <ol...@chromium.org>

Gerrit-Comment-Date: Fri, 24 Jul 2026 12:24:54 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Olivier Flückiger (Gerrit)]

Olivier Flückiger voted and added 5 comments

Votes added by Olivier Flückiger

Auto-Submit	+1
Commit-Queue	+2

5 comments

Patchset-level comments

File-level comment, Patchset 4:

Victor Gomes . resolved

Nice! LGTM % forcing ReduceResult

Olivier Flückiger

Done

File-level comment, Patchset 5 (Latest):

Olivier Flückiger . resolved

thanks

File src/maglev/maglev-reducer-inl.h

Line 1132, Patchset 4: ValueNode* length) -> MaybeReduceResult {

Victor Gomes . resolved

ReduceResult

Olivier Flückiger

Done

Line 940, Patchset 4: ValueNode* length) -> MaybeReduceResult {

Victor Gomes . resolved

ReduceResult

Olivier Flückiger

Done

Line 930, Patchset 4: return Reducer(elements_kind, elements, length);

Victor Gomes . resolved

Let's restrict Reducer to return a ReduceResult, not a MaybeReduceResult. Because, by this point we have already inserted map checks and emitted loads, we should not fail the reduction.

We can either do:
```
ReduceResult result = Reducer(elements_kind, elements, length);
return result;
```

or we can change the typename ReducerCb to base::FunctionRef<ReduceResult(...args...)

Up to you.

Olivier Flückiger

Done

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: Iade87e9f73c9d0ded8be6957dd7f36e8eef460d5

Gerrit-Change-Number: 8139484

Gerrit-PatchSet: 5

Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>

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

Gerrit-Comment-Date: Fri, 24 Jul 2026 12:49:21 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Victor Gomes <victo...@chromium.org>

[v8-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)]

v8-s...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes

Unreviewed changes

4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/maglev/maglev-reducer-inl.h
Insertions: 4, Deletions: 5.

@@ -927,7 +927,8 @@
   ValueNode* elements;
   GET_VALUE_OR_ABORT(elements, BuildLoadElements(receiver, elements_kind));
 
-  return Reducer(elements_kind, elements, length);
+  ReduceResult res = Reducer(elements_kind, elements, length);
+  return res;
 }
 
 template <typename BaseT>
@@ -936,8 +937,7 @@
     const char* builtin_name, CallArguments& args, ReducerCb Reducer) {
   return TryWithFastArrayElements(
       builtin_name, args,
-      [&](ElementsKind elements_kind, ValueNode* elements,
-          ValueNode* length) -> MaybeReduceResult {
+      [&](ElementsKind elements_kind, ValueNode* elements, ValueNode* length) {
         ValueNode* search_element =
             args.count() > 0 ? args[0]
                              : GetRootConstant(RootIndex::kUndefinedValue);
@@ -1128,8 +1128,7 @@
     compiler::JSFunctionRef target, CallArguments& args) {
   return TryWithFastArrayElements(
       "Array.prototype.at", args,
-      [&](ElementsKind elements_kind, ValueNode* elements,
-          ValueNode* length) -> MaybeReduceResult {
+      [&](ElementsKind elements_kind, ValueNode* elements, ValueNode* length) {
         ValueNode* index = nullptr;
         if (args.count() == 0) {
           // Index is the undefined object. ToIntegerOrInfinity(undefined) = 0.
```

Change information

Commit message:

[maglev] Refactor TryWithArrayIterationArgs into layered helpers

Extract TryWithFastArrayElements as a standalone foundational base
helper in MaglevReducer to handle fast array iteration checks and
operations.

Re-implement TryWithArrayIterationArgs to build directly on top of
TryWithFastArrayElements, separating argument search element extraction
and fromIndex clamping logic from core array setup.

Refactor TryReduceArrayPrototypeAt to use TryWithFastArrayElements,
eliminating its unconditional NoElementsProtector dependency on packed
arrays, ensuring IsArrayLength::kYes is passed when loading length,
and deduplicating initial receiver check boilerplate.

Bug: 42204525

TAG=agy
CONV=c99a500e-d220-44f7-a578-b5242feca754

Change-Id: Iade87e9f73c9d0ded8be6957dd7f36e8eef460d5

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

Commit-Queue: Olivier Flückiger <ol...@chromium.org>

Reviewed-by: Victor Gomes <victo...@chromium.org>

Auto-Submit: Olivier Flückiger <ol...@chromium.org>

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

Files:

• M src/maglev/maglev-reducer-inl.h
• M src/maglev/maglev-reducer.h

Change size: L

Delta: 2 files changed, 129 insertions(+), 134 deletions(-)

Branch: refs/heads/main

Submit Requirements:

• Code-Review: +1 by Victor Gomes

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: Iade87e9f73c9d0ded8be6957dd7f36e8eef460d5

Gerrit-Change-Number: 8139484

Gerrit-PatchSet: 6

Gerrit-Owner: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Olivier Flückiger <ol...@chromium.org>

Gerrit-Reviewer: Victor Gomes <victo...@chromium.org>

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