[compiler] Reduce I32x4DotI8x16S [v8/v8 : main]

0 views
Skip to first unread message

Matthias Liedtke (Gerrit)

unread,
Jul 22, 2026, 10:13:41 AM (3 days ago) Jul 22
to Sam Parker-Haynes, Thibaud Michaud, v8-s...@luci-project-accounts.iam.gserviceaccount.com, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Sam Parker-Haynes and Thibaud Michaud

Matthias Liedtke added 1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Matthias Liedtke . resolved

@thib...@chromium.org: Can I offload this change to reduce my review-load? I was out for a few days and I have a bunch of reviews in my pipeline.

Open in Gerrit

Related details

Attention is currently required from:
  • Sam Parker-Haynes
  • Thibaud Michaud
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I62ad3a9193fa2c751d9753849ebf52aa6f088681
Gerrit-Change-Number: 8092489
Gerrit-PatchSet: 2
Gerrit-Owner: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Reviewer: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>
Gerrit-CC: Matthias Liedtke <mlie...@chromium.org>
Gerrit-Attention: Thibaud Michaud <thib...@chromium.org>
Gerrit-Attention: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Comment-Date: Wed, 22 Jul 2026 14:13:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
open
diffy

Thibaud Michaud (Gerrit)

unread,
Jul 24, 2026, 10:19:10 AM (19 hours ago) Jul 24
to Sam Parker-Haynes, Matthias Liedtke, v8-s...@luci-project-accounts.iam.gserviceaccount.com, dmercadi...@chromium.org, v8-re...@googlegroups.com
Attention needed from Sam Parker-Haynes

Thibaud Michaud voted and added 4 comments

Votes added by Thibaud Michaud

Code-Review+1

4 comments

Patchset-level comments
Thibaud Michaud . resolved

Sorry for the wait, this is a pretty large change and I had to look at the original change that introduces I32x4DotI8x16S to understand the context better.
LGTM with a nit and some questions for my own understanding.

File src/compiler/turboshaft/wasm-shuffle-reducer.h
Line 370, Patchset 2 (Latest): // Represents one candidate dot-product reduction rooted at an
// I32x4AddReduce. The search walks backwards through a tree of I32x4Add,
// Phi, and I32x4DotI8x16S operations, collecting dots whose generated input
// shuffles can be removed.
Thibaud Michaud . unresolved

Could we explain the motivation behind this optimization here? I.e. that this is to undo the shuffle that we introduce in https://crrev.com/c/7956960.
There is such a comment at wasm-shuffle-reducer.cc:292 but I think it deserves to be more visible.

File src/compiler/turboshaft/wasm-shuffle-reducer.cc
Line 255, Patchset 2 (Latest): // Sub Dot / |
Thibaud Michaud . unresolved

In principle couldn't we also visit the inputs of Sub?
The elements of the input will either all be subtracted or all be added to the total, so shuffling them shouldn't affect the result.

Line 333, Patchset 2 (Latest): // We allow any other I32x4AddReduce operation that we didn't discover as
// part of the search.
if (const Simd128ReduceOp* reduce = use_op.TryCast<Simd128ReduceOp>()) {
if (reduce->kind == Simd128ReduceOp::Kind::kI32x4AddReduce) continue;
}
TRACE("Reduction node %d has outside user %d\n", node.id(), use.id());
return false;
Thibaud Michaud . unresolved

So if a Dot belongs to multiple trees, something like this:

```
A Dot B
\ / \ /
Add Add
| |
AddReduce AddReduce
```

The shuffle could be eliminated in principle, but we will not detect it in this case because the Dot does not flow directly into the other AddReduce, right?

Open in Gerrit

Related details

Attention is currently required from:
  • Sam Parker-Haynes
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: v8/v8
Gerrit-Branch: main
Gerrit-Change-Id: I62ad3a9193fa2c751d9753849ebf52aa6f088681
Gerrit-Change-Number: 8092489
Gerrit-PatchSet: 2
Gerrit-Owner: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Reviewer: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Reviewer: Thibaud Michaud <thib...@chromium.org>
Gerrit-CC: Matthias Liedtke <mlie...@chromium.org>
Gerrit-Attention: Sam Parker-Haynes <sam.p...@arm.com>
Gerrit-Comment-Date: Fri, 24 Jul 2026 14:19:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages