Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/ir/possible-constant.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,12 @@ struct PossibleConstantValues {
value = val.and_(Literal(uint32_t(0xffff)));
}
break;
case Field::WaitQueue:
WASM_UNREACHABLE("not implemented");
break;
case Field::not_packed:
WASM_UNREACHABLE("unexpected packed type");
break;
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/parser/contexts.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ struct NullTypeParserCtx {

StorageT makeI8() { return Ok{}; }
StorageT makeI16() { return Ok{}; }
StorageT makeWaitQueue() { return Ok{}; }
StorageT makeStorageType(TypeT) { return Ok{}; }

FieldT makeFieldType(StorageT, Mutability) { return Ok{}; }
Expand Down Expand Up @@ -308,6 +309,7 @@ template<typename Ctx> struct TypeParserCtx {

StorageT makeI8() { return Field(Field::i8, Immutable); }
StorageT makeI16() { return Field(Field::i16, Immutable); }
StorageT makeWaitQueue() { return Field(Field::WaitQueue, Immutable); }
StorageT makeStorageType(TypeT type) { return Field(type, Immutable); }

FieldT makeFieldType(FieldT field, Mutability mutability) {
Expand Down
4 changes: 4 additions & 0 deletions src/parser/parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,10 @@ template<typename Ctx> Result<typename Ctx::FieldT> storagetype(Ctx& ctx) {
if (ctx.in.takeKeyword("i16"sv)) {
return ctx.makeI16();
}
if (ctx.in.takeKeyword("waitqueue"sv)) {
return ctx.makeWaitQueue();
}

auto type = valtype(ctx);
CHECK_ERR(type);
return ctx.makeStorageType(*type);
Expand Down
5 changes: 3 additions & 2 deletions src/wasm-binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,9 @@ enum EncodedType {
f64 = -0x4, // 0x7c
v128 = -0x5, // 0x7b
// packed types
i8 = -0x8, // 0x78
i16 = -0x9, // 0x77
i8 = -0x8, // 0x78
i16 = -0x9, // 0x77
waitQueue = -0x24, // 0x5c
// reference types
nullfuncref = -0xd, // 0x73
nullexternref = -0xe, // 0x72
Expand Down
4 changes: 4 additions & 0 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -2752,6 +2752,10 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
memcpy(&i, p, sizeof(i));
return truncateForPacking(Literal(int32_t(i)), field);
}
case Field::WaitQueue: {
WASM_UNREACHABLE("waitqueue not implemented");
break;
}
}
WASM_UNREACHABLE("unexpected type");
}
Expand Down
1 change: 1 addition & 0 deletions src/wasm-type.h
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ struct Field {
not_packed,
i8,
i16,
WaitQueue,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks inconsistent with the existing values, in particular not_packed vs WaitQueue?

Which proposal is this from btw? I'd like to read the background.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The context is in the linked issue and in particular in Thomas's issue. I'm implementing the second paragraph in that issue which is WaitQueue as a packed type. Since I'm implementing it as a packed type, I think this is the right place for this but let me know if I got it wrong.

The problem that we're trying to solve is that WaitQueue needs to be partially opaque, but we'd also like existing struct accessors to be able to mutate its control word.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that you meant the naming. I don't mind changing it, but it looks like most enums values in this repo use PascalCase, maybe we should change not_packed to NotPacked instead? Either way is ok to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right, maybe it should be NotPacked. I guess we can leave that as separate from this PR then.

Reading some of the discussion on the substance, this seems... complicated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make a decision though on planning to rename to NotPacked, or something else. I think I'd be in favor of that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! Will change it in the next PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sg, though actually let's wait for @tlively to weigh in, as maybe there was a reason for not_packed that I don't recall...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotPacked sgtm

} packedType; // applicable iff type=i32
Mutability mutable_;

Expand Down
6 changes: 6 additions & 0 deletions src/wasm/wasm-binary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1996,6 +1996,8 @@ void WasmBinaryWriter::writeField(const Field& field) {
o << S32LEB(BinaryConsts::EncodedType::i8);
} else if (field.packedType == Field::i16) {
o << S32LEB(BinaryConsts::EncodedType::i16);
} else if (field.packedType == Field::WaitQueue) {
o << S32LEB(BinaryConsts::EncodedType::waitQueue);
} else {
WASM_UNREACHABLE("invalid packed type");
}
Expand Down Expand Up @@ -2717,6 +2719,10 @@ void WasmBinaryReader::readTypes() {
auto mutable_ = readMutability();
return Field(Field::i16, mutable_);
}
if (typeCode == BinaryConsts::EncodedType::waitQueue) {
auto mutable_ = readMutability();
return Field(Field::WaitQueue, mutable_);
}
// It's a regular wasm value.
auto type = makeType(typeCode);
auto mutable_ = readMutability();
Expand Down
5 changes: 5 additions & 0 deletions src/wasm/wasm-type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,9 @@ unsigned Field::getByteSize() const {
return 2;
case Field::PackedType::not_packed:
return 4;
case Field::PackedType::WaitQueue:
WASM_UNREACHABLE("waitqueue not implemented");
break;
}
WASM_UNREACHABLE("impossible packed type");
}
Expand Down Expand Up @@ -1874,6 +1877,8 @@ std::ostream& TypePrinter::print(const Field& field) {
os << "i8";
} else if (packedType == Field::PackedType::i16) {
os << "i16";
} else if (packedType == Field::PackedType::WaitQueue) {
os << "waitqueue";
} else {
WASM_UNREACHABLE("unexpected packed type");
}
Expand Down
9 changes: 9 additions & 0 deletions test/lit/waitqueue.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
;; RUN: wasm-opt %s -all --roundtrip -S -o - | filecheck %s --check-prefix=RTRIP
(module
;; RTRIP: (type $t (struct (field waitqueue)))
(type $t (struct (field waitqueue)))

;; use $t so roundtripping doesn't drop the definition
(global (ref null $t) (ref.null $t))
)
Loading