[wasm-gc] Remove br_on_func & br_on_non_func op [v8/v8 : main]
Matthias Liedtke (Gerrit)
Attention is currently required from: Jakob Kummerow.
Patch set 1:Commit-Queue +1
1 comment:
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
Patch Set #1, Line 5:
// Flags: --experimental-wasm-gcI don't have access to https://bugs.chromium.org/p/chromium/issues/detail?id=1339321.
I deleted the test case as I assumed it was related to `br_on_non_func` which is the main instruction in the `crash` function below.
Looking at https://chromium-review.googlesource.com/c/v8/v8/+/3724787 it doesn't seem like it. Should I try replacing the branch instruction with a different kind?
To view, visit change 3803226. To unsubscribe, or for help writing mail filters, visit settings.
Jakob Kummerow (Gerrit)
Attention is currently required from: Matthias Liedtke.
Patch set 1:Code-Review +1
2 comments:
Patchset:
LGTM % comment.
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
Patch Set #1, Line 5:
// Flags: --experimental-wasm-gc
I don't have access to https://bugs.chromium.org/p/chromium/issues/detail?id=1339321. […]
Yes, please use one of the other `br_on_foo` instructions instead of deleting the test.
To view, visit change 3803226. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Liedtke (Gerrit)
2 comments:
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
Patch Set #1, Line 5:
// Flags: --experimental-wasm-gc
Yes, please use one of the other `br_on_foo` instructions instead of deleting the test.
Done
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
kExprDrop,
kExprDrop,Why do we have 2 drops here?
One is for the `ref.func` in line 36 but what is the second drop for?
To view, visit change 3803226. To unsubscribe, or for help writing mail filters, visit settings.
Jakob Kummerow (Gerrit)
Attention is currently required from: Matthias Liedtke.
Patch set 2:Code-Review +1
1 comment:
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
kExprDrop,
kExprDrop,
Why do we have 2 drops here? […]
the `ref.func` in line 34. Since the loop's type indicates that it consumes a funcref, it's allowed to drop that here.
That said, please use another type of branch here, such as i31 or array or data (sorry for not being more specific earlier). `br_on_non_null` is already being tested in line 22; I believe this test is meant to provide coverage for the shared implementation of generic br_on_* (where Liftoff uses `AbstractTypeCheck`).
To view, visit change 3803226. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Liedtke (Gerrit)
Patch set 3:Commit-Queue +1
1 comment:
File test/mjsunit/regress/wasm/regress-crbug-1339321.js:
kExprDrop,
kExprDrop,
the `ref.func` in line 34. […]
Done
To view, visit change 3803226. To unsubscribe, or for help writing mail filters, visit settings.
Matthias Liedtke (Gerrit)
Patch set 3:Commit-Queue +2
V8 LUCI CQ (Gerrit)
V8 LUCI CQ submitted this change.
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: test/mjsunit/regress/wasm/regress-crbug-1339321.js
Insertions: 2, Deletions: 1.
@@ -34,7 +34,8 @@
kExprRefNull, kAnyRefCode,
kExprLoop, loop_type,
kExprRefNull, kAnyRefCode,
- kExprBrOnNonNull, 0,
+ kGCPrefix, kExprBrOnI31, 0,
+ kExprDrop,
kExprDrop,
kExprEnd, // loop
]);
```
[wasm-gc] Remove br_on_func & br_on_non_func op
Preparation step to remove the subtype relationship between funcref and anyref.
Bug: v8:7748
Change-Id: Ie4b29bfc4b874aaca668082018f5359d1b6e3a2e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3803226
Reviewed-by: Jakob Kummerow <jkum...@chromium.org>
Commit-Queue: Matthias Liedtke <mlie...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82125}
---
M src/compiler/wasm-compiler.cc
M src/compiler/wasm-compiler.h
M src/wasm/baseline/liftoff-compiler.cc
M src/wasm/function-body-decoder-impl.h
M src/wasm/graph-builder-interface.cc
M src/wasm/wasm-opcodes.h
M test/common/wasm/wasm-macro-gen.h
M test/mjsunit/regress/wasm/regress-crbug-1339321.js
M test/mjsunit/wasm/wasm-module-builder.js
M test/unittests/wasm/function-body-decoder-unittest.cc
10 files changed, 31 insertions(+), 84 deletions(-)