Skip to content

Commit 3b5a675

Browse files
kripkenkripken
authored
[Types] Handle function types fully in more places (#3381)
Call isFunction to check for a general function type instead of just a funcref, in places where we care about both, and some other minor miscellaneous typing fixes in preparation for typed function references (this will be tested fully at that time). Change is mostly whitespace.
1 parent 091a50c commit 3b5a675

File tree

5 files changed

+120
-101
lines changed

5 files changed

+120
-101
lines changed

‎src/passes/InstrumentLocals.cpp

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -134,44 +134,46 @@ struct InstrumentLocals : public WalkerPass<PostWalker<InstrumentLocals>> {
134134

135135
Builder builder(*getModule());
136136
Name import;
137-
TODO_SINGLE_COMPOUND(curr->value->type);
138-
switch (curr->value->type.getBasic()) {
139-
case Type::i32:
140-
import = set_i32;
141-
break;
142-
case Type::i64:
143-
return; // TODO
144-
case Type::f32:
145-
import = set_f32;
146-
break;
147-
case Type::f64:
148-
import = set_f64;
149-
break;
150-
case Type::v128:
151-
import = set_v128;
152-
break;
153-
case Type::funcref:
154-
import = set_funcref;
155-
break;
156-
case Type::externref:
157-
import = set_externref;
158-
break;
159-
case Type::exnref:
160-
import = set_exnref;
161-
break;
162-
case Type::anyref:
163-
import = set_anyref;
164-
break;
165-
case Type::eqref:
166-
import = set_eqref;
167-
break;
168-
case Type::i31ref:
169-
import = set_i31ref;
170-
break;
171-
case Type::unreachable:
172-
return; // nothing to do here
173-
case Type::none:
174-
WASM_UNREACHABLE("unexpected type");
137+
auto type = curr->value->type;
138+
if (type.isFunction()) {
139+
import = set_funcref;
140+
} else {
141+
TODO_SINGLE_COMPOUND(curr->value->type);
142+
switch (type.getBasic()) {
143+
case Type::i32:
144+
import = set_i32;
145+
break;
146+
case Type::i64:
147+
return; // TODO
148+
case Type::f32:
149+
import = set_f32;
150+
break;
151+
case Type::f64:
152+
import = set_f64;
153+
break;
154+
case Type::v128:
155+
import = set_v128;
156+
break;
157+
case Type::externref:
158+
import = set_externref;
159+
break;
160+
case Type::exnref:
161+
import = set_exnref;
162+
break;
163+
case Type::anyref:
164+
import = set_anyref;
165+
break;
166+
case Type::eqref:
167+
import = set_eqref;
168+
break;
169+
case Type::i31ref:
170+
import = set_i31ref;
171+
break;
172+
case Type::unreachable:
173+
return; // nothing to do here
174+
default:
175+
WASM_UNREACHABLE("unexpected type");
176+
}
175177
}
176178
curr->value = builder.makeCall(import,
177179
{builder.makeConst(int32_t(id++)),

‎src/wasm-builder.h

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -741,24 +741,29 @@ class Builder {
741741
// Make a constant expression. This might be a wasm Const, or something
742742
// else of constant value like ref.null.
743743
Expression* makeConstantExpression(Literal value) {
744-
TODO_SINGLE_COMPOUND(value.type);
745-
switch (value.type.getBasic()) {
746-
case Type::funcref:
747-
if (!value.isNull()) {
748-
return makeRefFunc(value.getFunc());
749-
}
750-
return makeRefNull(value.type);
744+
auto type = value.type;
745+
if (type.isNumber()) {
746+
return makeConst(value);
747+
}
748+
if (type.isFunction()) {
749+
if (!value.isNull()) {
750+
// TODO: with typed function references we need to do more for the type
751+
return makeRefFunc(value.getFunc());
752+
}
753+
return makeRefNull(type);
754+
}
755+
TODO_SINGLE_COMPOUND(type);
756+
switch (type.getBasic()) {
751757
case Type::externref:
752758
case Type::exnref: // TODO: ExceptionPackage?
753759
case Type::anyref:
754760
case Type::eqref:
755761
assert(value.isNull() && "unexpected non-null reference type literal");
756-
return makeRefNull(value.type);
762+
return makeRefNull(type);
757763
case Type::i31ref:
758764
return makeI31New(makeConst(value.geti31()));
759765
default:
760-
assert(value.type.isNumber());
761-
return makeConst(value);
766+
WASM_UNREACHABLE("invalid constant expression");
762767
}
763768
}
764769

@@ -923,6 +928,9 @@ class Builder {
923928
if (curr->type.isTuple()) {
924929
return makeConstantExpression(Literal::makeZeros(curr->type));
925930
}
931+
if (curr->type.isFunction()) {
932+
return ExpressionManipulator::refNull(curr, curr->type);
933+
}
926934
Literal value;
927935
// TODO: reuse node conditionally when possible for literals
928936
TODO_SINGLE_COMPOUND(curr->type);
@@ -946,6 +954,7 @@ class Builder {
946954
break;
947955
}
948956
case Type::funcref:
957+
WASM_UNREACHABLE("handled above");
949958
case Type::externref:
950959
case Type::exnref:
951960
case Type::anyref:

‎src/wasm/literal.cpp

Lines changed: 53 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -422,58 +422,59 @@ void Literal::printVec128(std::ostream& o, const std::array<uint8_t, 16>& v) {
422422

423423
std::ostream& operator<<(std::ostream& o, Literal literal) {
424424
prepareMinorColor(o);
425-
TODO_SINGLE_COMPOUND(literal.type);
426-
switch (literal.type.getBasic()) {
427-
case Type::none:
428-
o << "?";
429-
break;
430-
case Type::i32:
431-
o << literal.geti32();
432-
break;
433-
case Type::i64:
434-
o << literal.geti64();
435-
break;
436-
case Type::f32:
437-
literal.printFloat(o, literal.getf32());
438-
break;
439-
case Type::f64:
440-
literal.printDouble(o, literal.getf64());
441-
break;
442-
case Type::v128:
443-
o << "i32x4 ";
444-
literal.printVec128(o, literal.getv128());
445-
break;
446-
case Type::funcref:
447-
if (literal.isNull()) {
448-
o << "funcref(null)";
449-
} else {
450-
o << "funcref(" << literal.getFunc() << ")";
451-
}
452-
break;
453-
case Type::externref:
454-
assert(literal.isNull() && "unexpected non-null externref literal");
455-
o << "externref(null)";
456-
break;
457-
case Type::exnref:
458-
if (literal.isNull()) {
459-
o << "exnref(null)";
460-
} else {
461-
o << "exnref(" << literal.getExceptionPackage() << ")";
462-
}
463-
break;
464-
case Type::anyref:
465-
assert(literal.isNull() && "unexpected non-null anyref literal");
466-
o << "anyref(null)";
467-
break;
468-
case Type::eqref:
469-
assert(literal.isNull() && "unexpected non-null eqref literal");
470-
o << "eqref(null)";
471-
break;
472-
case Type::i31ref:
473-
o << "i31ref(" << literal.geti31() << ")";
474-
break;
475-
case Type::unreachable:
476-
WASM_UNREACHABLE("invalid type");
425+
if (literal.type.isFunction()) {
426+
if (literal.isNull()) {
427+
o << "funcref(null)";
428+
} else {
429+
o << "funcref(" << literal.getFunc() << ")";
430+
}
431+
} else {
432+
TODO_SINGLE_COMPOUND(literal.type);
433+
switch (literal.type.getBasic()) {
434+
case Type::none:
435+
o << "?";
436+
break;
437+
case Type::i32:
438+
o << literal.geti32();
439+
break;
440+
case Type::i64:
441+
o << literal.geti64();
442+
break;
443+
case Type::f32:
444+
literal.printFloat(o, literal.getf32());
445+
break;
446+
case Type::f64:
447+
literal.printDouble(o, literal.getf64());
448+
break;
449+
case Type::v128:
450+
o << "i32x4 ";
451+
literal.printVec128(o, literal.getv128());
452+
break;
453+
case Type::externref:
454+
assert(literal.isNull() && "unexpected non-null externref literal");
455+
o << "externref(null)";
456+
break;
457+
case Type::exnref:
458+
if (literal.isNull()) {
459+
o << "exnref(null)";
460+
} else {
461+
o << "exnref(" << literal.getExceptionPackage() << ")";
462+
}
463+
break;
464+
case Type::anyref:
465+
assert(literal.isNull() && "unexpected non-null anyref literal");
466+
o << "anyref(null)";
467+
break;
468+
case Type::eqref:
469+
assert(literal.isNull() && "unexpected non-null eqref literal");
470+
o << "eqref(null)";
471+
break;
472+
case Type::i31ref:
473+
o << "i31ref(" << literal.geti31() << ")";
474+
break;
475+
default:
476+
WASM_UNREACHABLE("invalid type");
477+
}
477478
}
478479
restoreNormalColor(o);
479480
return o;

‎src/wasm/wasm-type.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,9 @@ Type Type::getLeastUpperBound(Type a, Type b) {
579579
}
580580
if (a.isRef()) {
581581
if (b.isRef()) {
582+
if (a.isFunction() && b.isFunction()) {
583+
return Type::funcref;
584+
}
582585
if ((a == Type::i31ref && b == Type::eqref) ||
583586
(a == Type::eqref && b == Type::i31ref)) {
584587
return Type::eqref;

‎src/wasm/wasm-validator.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2030,6 +2030,10 @@ void FunctionValidator::visitRefFunc(RefFunc* curr) {
20302030
}
20312031
auto* func = getModule()->getFunctionOrNull(curr->func);
20322032
shouldBeTrue(!!func, curr, "function argument of ref.func must exist");
2033+
shouldBeTrue(curr->type.isFunction(),
2034+
curr,
2035+
"ref.func must have a function reference type");
2036+
// TODO: check for non-nullability
20332037
}
20342038

20352039
void FunctionValidator::visitRefEq(RefEq* curr) {
@@ -2311,7 +2315,7 @@ void FunctionValidator::visitFunction(Function* curr) {
23112315
shouldBeTrue(var.isConcrete(), curr, "vars must be concretely typed");
23122316
}
23132317
shouldBeTrue(features <= getModule()->features,
2314-
curr,
2318+
curr->name,
23152319
"all used types should be allowed");
23162320
if (curr->profile == IRProfile::Poppy) {
23172321
shouldBeTrue(

0 commit comments

Comments
 (0)