Skip to content

Commit 2b43121

Browse files
jakobkummerowV8 LUCI CQ
jakobkummerow
authored and
V8 LUCI CQ
committed
[wasm] Increase maximum number of canonicalized types
By no longer encoding canonicalized types in {ValueType}s, we can support more of them. Change-Id: I0bdb0a3319e422dad76d7a0bbd03001d25346690 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5615591 Commit-Queue: Jakob Kummerow <[email protected]> Reviewed-by: Clemens Backes <[email protected]> Cr-Commit-Position: refs/heads/main@{#94368}
1 parent 718d9b6 commit 2b43121

11 files changed

Lines changed: 82 additions & 65 deletions

File tree

‎src/builtins/wasm.tq‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ extern runtime WasmStringViewWtf8Slice(Context, ByteArray, Number, Number):
7676
extern runtime WasmStringFromCodePoint(Context, Number): String;
7777
extern runtime WasmStringHash(NoContext, String): Smi;
7878
extern runtime WasmSubstring(Context, String, Smi, Smi): String;
79-
extern runtime WasmJSToWasmObject(Context, JSAny, Smi): JSAny;
79+
extern runtime WasmJSToWasmObject(Context, JSAny, Smi, Smi): JSAny;
8080

8181
extern runtime PropagateException(NoContext): JSAny;
8282
}
@@ -1492,7 +1492,7 @@ builtin WasmAnyConvertExtern(externObject: JSAny): JSAny {
14921492
const context = LoadContextFromInstanceData(trustedData);
14931493

14941494
tail runtime::WasmJSToWasmObject(
1495-
context, externObject, SmiConstant(kAnyType));
1495+
context, externObject, SmiConstant(kAnyType), SmiConstant(0));
14961496
}
14971497

14981498
extern macro CallOrConstructBuiltinsAssembler::GetCompatibleReceiver(

‎src/compiler/wasm-compiler.cc‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7360,20 +7360,22 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
73607360
// Make sure ValueType fits in a Smi.
73617361
static_assert(wasm::ValueType::kLastUsedBit + 1 <= kSmiValueSize);
73627362

7363+
uint32_t canonical_index = wasm::kInvalidCanonicalIndex;
73637364
if (type.has_index()) {
73647365
DCHECK_NOT_NULL(module);
7365-
uint32_t canonical_index =
7366+
canonical_index =
73667367
module->isorecursive_canonical_type_ids[type.ref_index()];
7367-
type = wasm::ValueType::RefMaybeNull(canonical_index,
7368-
type.nullability());
7368+
DCHECK_LE(canonical_index, kSmiMaxValue);
73697369
}
73707370

7371-
Node* inputs[] = {
7372-
input, mcgraph()->IntPtrConstant(
7373-
IntToSmi(static_cast<int>(type.raw_bit_field())))};
7371+
Node* inputs[] = {input,
7372+
mcgraph()->IntPtrConstant(IntToSmi(
7373+
static_cast<int>(type.raw_bit_field()))),
7374+
mcgraph()->IntPtrConstant(
7375+
IntToSmi(static_cast<int>(canonical_index)))};
73747376

73757377
return BuildCallToRuntimeWithContext(Runtime::kWasmJSToWasmObject,
7376-
js_context, inputs, 2);
7378+
js_context, inputs, 3);
73777379
}
73787380
}
73797381
}

‎src/runtime/runtime-wasm.cc‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -162,46 +162,49 @@ RUNTIME_FUNCTION(Runtime_WasmGenericJSToWasmObject) {
162162
int raw_type = args.smi_value_at(2);
163163

164164
wasm::ValueType type = wasm::ValueType::FromRawBitField(raw_type);
165+
uint32_t canonical_index = wasm::kInvalidCanonicalIndex;
165166
if (type.has_index()) {
166167
DirectHandle<WasmTrustedInstanceData> trusted_instance_data(
167168
WasmTrustedInstanceData::cast(args[0]), isolate);
168169
const wasm::WasmModule* module = trusted_instance_data->module();
169170
DCHECK_NOT_NULL(module);
170-
uint32_t canonical_index =
171-
module->isorecursive_canonical_type_ids[type.ref_index()];
172-
type = wasm::ValueType::RefMaybeNull(canonical_index, type.nullability());
171+
canonical_index = module->isorecursive_canonical_type_ids[type.ref_index()];
173172
}
174173
const char* error_message;
175174
Handle<Object> result;
176-
if (!JSToWasmObject(isolate, value, type, &error_message).ToHandle(&result)) {
175+
if (!JSToWasmObject(isolate, value, type, canonical_index, &error_message)
176+
.ToHandle(&result)) {
177177
return isolate->Throw(*isolate->factory()->NewTypeError(
178178
MessageTemplate::kWasmTrapJSTypeError));
179179
}
180180
return *result;
181181
}
182182

183-
// Takes a JS object and a wasm type as Smi. Type checks the object against the
184-
// type; if the check succeeds, returns the object in its wasm representation;
185-
// otherwise throws a type error.
183+
// Parameters:
184+
// args[0]: the object, any JS value.
185+
// args[1]: the expected ValueType, Smi-tagged.
186+
// args[2]: the expected canonical type index, Smi-tagged, if the type is
187+
// an indexed reftype.
188+
// Type checks the object against the type; if the check succeeds, returns the
189+
// object in its wasm representation; otherwise throws a type error.
186190
RUNTIME_FUNCTION(Runtime_WasmJSToWasmObject) {
187191
// TODO(manoskouk): Use {SaveAndClearThreadInWasmFlag} in runtime-internal.cc
188192
// and runtime-strings.cc.
189193
bool thread_in_wasm = trap_handler::IsThreadInWasm();
190194
if (thread_in_wasm) trap_handler::ClearThreadInWasm();
191195
HandleScope scope(isolate);
192-
DCHECK_EQ(2, args.length());
193-
// 'raw_instance' can be either a WasmInstanceObject or undefined.
196+
DCHECK_EQ(3, args.length());
194197
Handle<Object> value(args[0], isolate);
195198
// Make sure ValueType fits properly in a Smi.
196199
static_assert(wasm::ValueType::kLastUsedBit + 1 <= kSmiValueSize);
197200
int raw_type = args.smi_value_at(1);
201+
int canonical_index = args.smi_value_at(2);
198202

199-
wasm::ValueType expected_canonical =
200-
wasm::ValueType::FromRawBitField(raw_type);
203+
wasm::ValueType expected = wasm::ValueType::FromRawBitField(raw_type);
201204
const char* error_message;
202205
Handle<Object> result;
203206
bool success =
204-
JSToWasmObject(isolate, value, expected_canonical, &error_message)
207+
JSToWasmObject(isolate, value, expected, canonical_index, &error_message)
205208
.ToHandle(&result);
206209
Tagged<Object> ret = success
207210
? *result

‎src/runtime/runtime.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ namespace internal {
652652
F(WasmTableCopy, 6, 1) \
653653
F(WasmTableGrow, 3, 1) \
654654
F(WasmTableFill, 5, 1) \
655-
F(WasmJSToWasmObject, 2, 1) \
655+
F(WasmJSToWasmObject, 3, 1) \
656656
F(WasmGenericJSToWasmObject, 3, 1) \
657657
F(WasmGenericWasmToJSObject, 1, 1) \
658658
F(WasmCompileLazy, 2, 1) \

‎src/wasm/canonical-types.cc‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ TypeCanonicalizer::TypeCanonicalizer() {
2222
AddPredefinedArrayType(kPredefinedArrayI16Index, kWasmI16);
2323
}
2424

25-
// We currently store canonical indices in {ValueType} instances, so they
26-
// must fit into the range of valid module-relative (non-canonical) type
27-
// indices.
28-
// TODO(jkummerow): Raise this limit, to make long-lived WasmEngines scale
29-
// better. Plan: stop constructing ValueTypes from canonical type indices.
30-
static constexpr size_t kMaxCanonicalTypes = kV8MaxWasmTypes;
25+
// For convenience, limit canonicalized type indices to Smi range.
26+
// We could squeeze out a few more bits if necessary by passing them
27+
// from compiled wrappers to runtime functions as Smi-tagged unsigned ints.
28+
// That would give us "uint31" range on 32-bit platforms, and allow
29+
// uint32_t (or even more) on 64-bit platforms. But we probably don't want
30+
// to store that many types in the TypeCanonicalizer anyway.
31+
static constexpr size_t kMaxCanonicalTypes = kSmiMaxValue;
32+
static_assert(kMaxCanonicalTypes >= kV8MaxWasmTypes);
33+
static_assert(kInvalidCanonicalIndex > kMaxCanonicalTypes);
3134

3235
void TypeCanonicalizer::CheckMaxCanonicalIndex() const {
3336
if (canonical_supertypes_.size() > kMaxCanonicalTypes) {
@@ -154,6 +157,7 @@ uint32_t TypeCanonicalizer::AddRecursiveGroup(const FunctionSig* sig) {
154157
group.type.is_relative_supertype = false;
155158
int canonical_index = FindCanonicalGroup(group);
156159
if (canonical_index >= 0) return canonical_index;
160+
static_assert(kMaxCanonicalTypes <= kMaxInt);
157161
canonical_index = static_cast<int>(canonical_supertypes_.size());
158162
// We need to copy the signature in the local zone, or else we risk
159163
// storing a dangling pointer in the future.
@@ -303,6 +307,7 @@ int TypeCanonicalizer::FindCanonicalGroup(const CanonicalGroup& group) const {
303307
int TypeCanonicalizer::FindCanonicalGroup(
304308
const CanonicalSingletonGroup& group) const {
305309
auto it = canonical_singleton_groups_.find(group);
310+
static_assert(kMaxCanonicalTypes <= kMaxInt);
306311
return it == canonical_singleton_groups_.end() ? -1 : it->second;
307312
}
308313

‎src/wasm/constant-expression-interface.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ void ConstantExpressionInterface::UnOp(FullDecoder* decoder, WasmOpcode opcode,
5656
case kExprAnyConvertExtern: {
5757
const char* error_message = nullptr;
5858
result->runtime_value = WasmValue(
59-
JSToWasmObject(isolate_, input.runtime_value.to_ref(), kWasmAnyRef,
59+
JSToWasmObject(isolate_, input.runtime_value.to_ref(), kWasmAnyRef, 0,
6060
&error_message)
6161
.ToHandleChecked(),
6262
ValueType::RefMaybeNull(HeapType::kAny, input.type.nullability()));

‎src/wasm/wasm-constants.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,13 @@ constexpr uint32_t kExceptionAttribute = 0;
171171

172172
constexpr int kAnonymousFuncIndex = -1;
173173

174+
// This needs to survive round-tripping through a Smi without changing
175+
// its value.
176+
constexpr uint32_t kInvalidCanonicalIndex = static_cast<uint32_t>(-1);
177+
static_assert(static_cast<uint32_t>(Internals::SmiValue(Internals::IntToSmi(
178+
static_cast<int>(kInvalidCanonicalIndex)))) ==
179+
kInvalidCanonicalIndex);
180+
174181
// The number of calls to an exported Wasm function that will be handled
175182
// by the generic wrapper. Once the budget is exhausted, a specific wrapper
176183
// is to be compiled for the function's signature.

‎src/wasm/wasm-js.cc‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,8 +1763,9 @@ void WebAssemblyGlobalImpl(const v8::FunctionCallbackInfo<v8::Value>& info) {
17631763
// no way to specify such types in `new WebAssembly.Global(...)`.
17641764
// TODO(14034): Fix this if that changes.
17651765
DCHECK(!type.has_index());
1766+
uint32_t unused_canonical_index = 0;
17661767
if (!i::wasm::JSToWasmObject(i_isolate, value_handle, type,
1767-
&error_message)
1768+
unused_canonical_index, &error_message)
17681769
.ToHandle(&value_handle)) {
17691770
thrower.TypeError("%s", error_message);
17701771
break;
@@ -1974,21 +1975,19 @@ void EncodeExceptionValues(
19741975
case i::wasm::kRefNull: {
19751976
const char* error_message;
19761977
i::Handle<i::Object> value_handle = Utils::OpenHandle(*value);
1977-
1978+
uint32_t canonical_index = i::wasm::kInvalidCanonicalIndex;
19781979
if (type.has_index()) {
19791980
// Canonicalize the type using the tag's original module.
19801981
i::Tagged<i::HeapObject> maybe_instance = tag_object->instance();
1981-
CHECK(!i::IsUndefined(maybe_instance));
1982+
// Indexed types are guaranteed to come from an instance.
1983+
CHECK(i::IsWasmInstanceObject(maybe_instance));
19821984
auto instance = i::WasmInstanceObject::cast(maybe_instance);
19831985
const i::wasm::WasmModule* module = instance->module();
1984-
uint32_t canonical_index =
1986+
canonical_index =
19851987
module->isorecursive_canonical_type_ids[type.ref_index()];
1986-
type = i::wasm::ValueType::RefMaybeNull(canonical_index,
1987-
type.nullability());
19881988
}
1989-
1990-
if (!internal::wasm::JSToWasmObject(i_isolate, value_handle, type,
1991-
&error_message)
1989+
if (!i::wasm::JSToWasmObject(i_isolate, value_handle, type,
1990+
canonical_index, &error_message)
19921991
.ToHandle(&value_handle)) {
19931992
thrower->TypeError("%s", error_message);
19941993
return;

‎src/wasm/wasm-objects.cc‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2580,11 +2580,11 @@ Handle<Object> CanonicalizeSmi(Handle<Object> smi, Isolate* isolate) {
25802580

25812581
namespace wasm {
25822582
MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
2583-
ValueType expected_canonical,
2583+
ValueType expected, uint32_t canonical_index,
25842584
const char** error_message) {
2585-
DCHECK(expected_canonical.is_object_reference());
2586-
if (expected_canonical.kind() == kRefNull && IsNull(*value, isolate)) {
2587-
switch (expected_canonical.heap_representation()) {
2585+
DCHECK(expected.is_object_reference());
2586+
if (expected.kind() == kRefNull && IsNull(*value, isolate)) {
2587+
switch (expected.heap_representation()) {
25882588
case HeapType::kStringViewWtf8:
25892589
*error_message = "stringview_wtf8 has no JS representation";
25902590
return {};
@@ -2602,7 +2602,7 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
26022602
return {};
26032603
default: {
26042604
HeapType::Representation repr =
2605-
expected_canonical.heap_representation_non_shared();
2605+
expected.heap_representation_non_shared();
26062606
bool is_extern_subtype =
26072607
repr == HeapType::kExtern || repr == HeapType::kNoExtern ||
26082608
repr == HeapType::kExn || repr == HeapType::kNoExn;
@@ -2611,7 +2611,7 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
26112611
}
26122612
}
26132613

2614-
switch (expected_canonical.heap_representation_non_shared()) {
2614+
switch (expected.heap_representation_non_shared()) {
26152615
case HeapType::kFunc: {
26162616
if (!(WasmExternalFunction::IsWasmExternalFunction(*value) ||
26172617
WasmCapiFunction::IsWasmCapiFunction(*value))) {
@@ -2707,24 +2707,24 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
27072707
}
27082708
default: {
27092709
auto type_canonicalizer = GetWasmEngine()->type_canonicalizer();
2710+
DCHECK_NE(canonical_index, kInvalidCanonicalIndex);
27102711

27112712
if (WasmExportedFunction::IsWasmExportedFunction(*value)) {
27122713
Tagged<WasmExportedFunction> function =
27132714
WasmExportedFunction::cast(*value);
27142715
uint32_t real_type_index = function->shared()
27152716
->wasm_exported_function_data()
27162717
->canonical_type_index();
2717-
if (!type_canonicalizer->IsCanonicalSubtype(
2718-
real_type_index, expected_canonical.ref_index())) {
2718+
if (!type_canonicalizer->IsCanonicalSubtype(real_type_index,
2719+
canonical_index)) {
27192720
*error_message =
27202721
"assigned exported function has to be a subtype of the "
27212722
"expected type";
27222723
return {};
27232724
}
27242725
return handle(WasmExternalFunction::cast(*value)->func_ref(), isolate);
27252726
} else if (WasmJSFunction::IsWasmJSFunction(*value)) {
2726-
if (!WasmJSFunction::cast(*value)->MatchesSignature(
2727-
expected_canonical.ref_index())) {
2727+
if (!WasmJSFunction::cast(*value)->MatchesSignature(canonical_index)) {
27282728
*error_message =
27292729
"assigned WebAssembly.Function has to be a subtype of the "
27302730
"expected type";
@@ -2733,7 +2733,7 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
27332733
return handle(WasmExternalFunction::cast(*value)->func_ref(), isolate);
27342734
} else if (WasmCapiFunction::IsWasmCapiFunction(*value)) {
27352735
if (!WasmCapiFunction::cast(*value)->MatchesSignature(
2736-
expected_canonical.ref_index())) {
2736+
canonical_index)) {
27372737
*error_message =
27382738
"assigned C API function has to be a subtype of the expected "
27392739
"type";
@@ -2748,8 +2748,8 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
27482748
WasmInstanceObject::cast(type_info->instance())->module();
27492749
uint32_t real_canonical_index =
27502750
real_module->isorecursive_canonical_type_ids[real_idx];
2751-
if (!type_canonicalizer->IsCanonicalSubtype(
2752-
real_canonical_index, expected_canonical.ref_index())) {
2751+
if (!type_canonicalizer->IsCanonicalSubtype(real_canonical_index,
2752+
canonical_index)) {
27532753
*error_message = "object is not a subtype of expected type";
27542754
return {};
27552755
}
@@ -2766,14 +2766,13 @@ MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
27662766
MaybeHandle<Object> JSToWasmObject(Isolate* isolate, const WasmModule* module,
27672767
Handle<Object> value, ValueType expected,
27682768
const char** error_message) {
2769-
ValueType expected_canonical = expected;
2770-
if (expected_canonical.has_index()) {
2771-
uint32_t canonical_index =
2772-
module->isorecursive_canonical_type_ids[expected_canonical.ref_index()];
2773-
expected_canonical = ValueType::RefMaybeNull(
2774-
canonical_index, expected_canonical.nullability());
2775-
}
2776-
return JSToWasmObject(isolate, value, expected_canonical, error_message);
2769+
uint32_t canonical_index = kInvalidCanonicalIndex;
2770+
if (expected.has_index()) {
2771+
canonical_index =
2772+
module->isorecursive_canonical_type_ids[expected.ref_index()];
2773+
}
2774+
return JSToWasmObject(isolate, value, expected, canonical_index,
2775+
error_message);
27772776
}
27782777

27792778
Handle<Object> WasmToJSObject(Isolate* isolate, Handle<Object> value) {

‎src/wasm/wasm-objects.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ namespace wasm {
13091309
// {expected}. If the typecheck succeeds, returns the wasm representation of the
13101310
// object; otherwise, returns the empty handle.
13111311
MaybeHandle<Object> JSToWasmObject(Isolate* isolate, Handle<Object> value,
1312-
ValueType expected_canonical,
1312+
ValueType expected, uint32_t canonical_index,
13131313
const char** error_message);
13141314

13151315
// Utility which canonicalizes {expected} in addition.

0 commit comments

Comments
 (0)