Skip to content

Commit 6a64cbc

Browse files
Always use LaneResults for SIMD values in spec tests (#8434)
For use in #8421. Previously LaneResults was only used if the expected v128 constant contained NaN patterns, which meant that we lose the shape information (otherwise they were stored/matched as a normal v128 Literal). Change the parser to always use LaneResult so that we keep the shape information and can output exactly which lane doesn't match during test failures. Error message before: ``` expected i32x4 0x56485649 0x56485648 0x56485648 0x56485648, got i32x4 0x56485648 0x56485648 0x56485648 0x56485648 ``` Error message after: ``` expected 22089, got 22088 at lane 0 ``` Part of #8261 and #8315.
1 parent f1062b0 commit 6a64cbc

File tree

5 files changed

+138
-47
lines changed

5 files changed

+138
-47
lines changed

src/parser/lexer.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,15 @@ void Lexer::skipSpace() {
935935
}
936936
}
937937

938+
std::optional<char> Lexer::peekChar() const {
939+
auto n = next();
940+
if (n.empty()) {
941+
return std::nullopt;
942+
}
943+
944+
return n[0];
945+
}
946+
938947
bool Lexer::takeLParen() {
939948
if (LexCtx(next()).startsWith("("sv)) {
940949
++pos;

src/parser/lexer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ struct Lexer {
7878
advance();
7979
}
8080

81+
std::optional<char> peekChar() const;
82+
8183
bool takeLParen();
8284

8385
bool peekLParen() { return Lexer(*this).takeLParen(); }

src/parser/wast-parser.cpp

Lines changed: 83 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -169,38 +169,63 @@ Result<NaNKind> nan(Lexer& in) {
169169
}
170170

171171
Result<ExpectedResult> result(Lexer& in) {
172-
Lexer constLexer = in;
173-
auto c = const_(constLexer);
174-
// TODO: Generating and discarding errors like this can lead to quadratic
175-
// behavior. Optimize this if necessary.
176-
if (!c.getErr()) {
177-
in = constLexer;
178-
return *c;
179-
}
180-
181-
// If we failed to parse a constant, we must have either a nan pattern or a
182-
// reference.
183-
if (in.takeSExprStart("f32.const"sv)) {
184-
auto kind = nan(in);
185-
CHECK_ERR(kind);
186-
if (!in.takeRParen()) {
187-
return in.err("expected end of f32.const");
172+
if (in.takeSExprStart("v128.const"sv)) {
173+
auto laneTypeChar = in.peekChar();
174+
if (!laneTypeChar) {
175+
return in.err("expected vector shape");
188176
}
189-
return NaNResult{*kind, Type::f32};
190-
}
191177

192-
if (in.takeSExprStart("f64.const"sv)) {
193-
auto kind = nan(in);
194-
CHECK_ERR(kind);
195-
if (!in.takeRParen()) {
196-
return in.err("expected end of f64.const");
178+
LaneResults::LaneType laneType;
179+
switch (*laneTypeChar) {
180+
case 'f': {
181+
laneType = LaneResults::LaneType::Float;
182+
break;
183+
}
184+
case 'i': {
185+
laneType = LaneResults::LaneType::Int;
186+
break;
187+
}
188+
default: {
189+
return in.err("expected vector shape");
190+
}
197191
}
198-
return NaNResult{*kind, Type::f64};
199-
}
200192

201-
if (in.takeSExprStart("v128.const"sv)) {
202-
LaneResults lanes;
203-
if (in.takeKeyword("f32x4"sv)) {
193+
LaneResults results(laneType);
194+
auto& lanes = results.lanes;
195+
196+
if (in.takeKeyword("i8x16"sv)) {
197+
for (int i = 0; i < 16; ++i) {
198+
auto int_ = in.takeI8();
199+
if (!int_) {
200+
return in.err("expected i8 immediate");
201+
}
202+
lanes.push_back(Literal(static_cast<uint32_t>(*int_)));
203+
}
204+
} else if (in.takeKeyword("i16x8"sv)) {
205+
for (int i = 0; i < 8; ++i) {
206+
auto int_ = in.takeI16();
207+
if (!int_) {
208+
return in.err("expected i16 immediate");
209+
}
210+
lanes.push_back(Literal(static_cast<uint32_t>(*int_)));
211+
}
212+
} else if (in.takeKeyword("i32x4"sv)) {
213+
for (int i = 0; i < 4; ++i) {
214+
auto int_ = in.takeI32();
215+
if (!int_) {
216+
return in.err("expected i32 immediate");
217+
}
218+
lanes.push_back(Literal(static_cast<uint32_t>(*int_)));
219+
}
220+
} else if (in.takeKeyword("i64x2"sv)) {
221+
for (int i = 0; i < 2; ++i) {
222+
auto int_ = in.takeI64();
223+
if (!int_) {
224+
return in.err("expected i64 immediate");
225+
}
226+
lanes.push_back(Literal(*int_));
227+
}
228+
} else if (in.takeKeyword("f32x4"sv)) {
204229
for (int i = 0; i < 4; ++i) {
205230
if (auto f = in.takeF32()) {
206231
lanes.push_back(Literal(*f));
@@ -226,7 +251,36 @@ Result<ExpectedResult> result(Lexer& in) {
226251
if (!in.takeRParen()) {
227252
return in.err("expected end of v128.const");
228253
}
229-
return lanes;
254+
return results;
255+
}
256+
257+
Lexer constLexer = in;
258+
auto c = const_(constLexer);
259+
// TODO: Generating and discarding errors like this can lead to quadratic
260+
// behavior. Optimize this if necessary.
261+
if (!c.getErr()) {
262+
in = constLexer;
263+
return *c;
264+
}
265+
266+
// If we failed to parse a constant, we must have either a nan pattern or a
267+
// reference.
268+
if (in.takeSExprStart("f32.const"sv)) {
269+
auto kind = nan(in);
270+
CHECK_ERR(kind);
271+
if (!in.takeRParen()) {
272+
return in.err("expected end of f32.const");
273+
}
274+
return NaNResult{*kind, Type::f32};
275+
}
276+
277+
if (in.takeSExprStart("f64.const"sv)) {
278+
auto kind = nan(in);
279+
CHECK_ERR(kind);
280+
if (!in.takeRParen()) {
281+
return in.err("expected end of f64.const");
282+
}
283+
return NaNResult{*kind, Type::f64};
230284
}
231285

232286
if (in.takeSExprStart("ref.null")) {

src/parser/wat-parser.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,17 @@ struct NaNResult {
7474

7575
using LaneResult = std::variant<Literal, NaNResult>;
7676

77-
using LaneResults = std::vector<LaneResult>;
77+
struct LaneResults {
78+
enum class LaneType {
79+
Int,
80+
Float,
81+
};
82+
LaneResults(LaneType type, std::vector<LaneResult> lanes = {})
83+
: type(type), lanes(std::move(lanes)) {}
84+
85+
LaneType type;
86+
std::vector<LaneResult> lanes;
87+
};
7888

7989
using ExpectedResult =
8090
std::variant<Literal, NullRefResult, RefResult, NaNResult, LaneResults>;

src/tools/wasm-shell.cpp

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -400,28 +400,44 @@ struct Shell {
400400
err << e->msg << atIndex();
401401
return Err{err.str()};
402402
}
403-
} else if (auto* lanes = std::get_if<LaneResults>(&expected)) {
403+
} else if (auto* l = std::get_if<LaneResults>(&expected)) {
404+
auto* lanes = &l->lanes;
405+
406+
auto check = [&](const auto& vals) -> Result<> {
407+
for (size_t i = 0; i < vals.size(); ++i) {
408+
auto check = checkLane(vals[i], (*lanes)[i], i);
409+
if (auto* e = check.getErr()) {
410+
err << e->msg << atIndex();
411+
return Err{err.str()};
412+
}
413+
}
414+
return Ok{};
415+
};
416+
417+
bool isFloat = l->type == WATParser::LaneResults::LaneType::Float;
404418
switch (lanes->size()) {
419+
// Use unsigned values for the smaller types here to avoid sign
420+
// extension when storing 8/16-bit values in 32-bit ints. This isn't
421+
// needed for i32 and i64.
422+
case 16: {
423+
// There is no f8.
424+
assert(!isFloat && "float8 does not exist");
425+
CHECK_ERR(check(val.getLanesUI8x16()));
426+
break;
427+
}
428+
case 8: {
429+
CHECK_ERR(
430+
check(isFloat ? val.getLanesF16x8() : val.getLanesUI16x8()));
431+
break;
432+
}
405433
case 4: {
406-
auto vals = val.getLanesF32x4();
407-
for (Index i = 0; i < 4; ++i) {
408-
auto check = checkLane(vals[i], (*lanes)[i], i);
409-
if (auto* e = check.getErr()) {
410-
err << e->msg << atIndex();
411-
return Err{err.str()};
412-
}
413-
}
434+
CHECK_ERR(
435+
check(isFloat ? val.getLanesF32x4() : val.getLanesI32x4()));
414436
break;
415437
}
416438
case 2: {
417-
auto vals = val.getLanesF64x2();
418-
for (Index i = 0; i < 2; ++i) {
419-
auto check = checkLane(vals[i], (*lanes)[i], i);
420-
if (auto* e = check.getErr()) {
421-
err << e->msg << atIndex();
422-
return Err{err.str()};
423-
}
424-
}
439+
CHECK_ERR(
440+
check(isFloat ? val.getLanesF64x2() : val.getLanesI64x2()));
425441
break;
426442
}
427443
default:

0 commit comments

Comments
 (0)