Commit f8973f1c authored by bradnelson's avatar bradnelson Committed by Commit bot

Revert of [wasm][asm.js] Fix and enable several asm.js tests with the new...

Revert of [wasm][asm.js] Fix and enable several asm.js tests with the new parser. (patchset #10 id:180001 of https://codereview.chromium.org/2771183002/ )

Reason for revert:
Fails on gc-stress.

Original issue's description:
> [wasm][asm.js] Fix and enable several asm.js tests with the new parser.
>
> Fix a few items broken during review of scanner + parser:
> * Make the scanner retain stale newline state on a rewind (as otherwise it must be able to correctly rewind that too, though it doesn't need it). (Probably should revisit).
> * Change StashCode in the builder skip to the zero case, as it crashes for some reason (added TODO).
>
> Also fix:
> * Drop test based on constant expression evaluation in main parser
> * Support constant defined based on existing constant.
> * Type constants as signed.
> * Added a check that all used functions are defined eventually.
> * Zone allocate strings for simplicity (TODOs to refactor better).
>
> BUG=v8:6090
> BUG=v8:4203
> R=mstarzinger@chromium.org,marja@chromium.org,vogelheim@chromium.org
>
> Review-Url: https://codereview.chromium.org/2771183002
> Cr-Commit-Position: refs/heads/master@{#44200}
> Committed: https://chromium.googlesource.com/v8/v8/+/be0dbdd679b60c31d480d7635e579787a6a218df

TBR=clemensh@chromium.org,bradnelson@google.com,marja@chromium.org,mstarzinger@chromium.org,vogelheim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:6090

Review-Url: https://codereview.chromium.org/2782613002
Cr-Commit-Position: refs/heads/master@{#44201}
parent be0dbdd6
...@@ -238,25 +238,21 @@ uint32_t AsmJsParser::VarIndex(VarInfo* info) { ...@@ -238,25 +238,21 @@ uint32_t AsmJsParser::VarIndex(VarInfo* info) {
void AsmJsParser::AddGlobalImport(std::string name, AsmType* type, void AsmJsParser::AddGlobalImport(std::string name, AsmType* type,
ValueType vtype, bool mutable_variable, ValueType vtype, bool mutable_variable,
VarInfo* info) { VarInfo* info) {
// TODO(bradnelson): Refactor memory management here.
// AsmModuleBuilder should really own import names.
char* name_data = zone()->NewArray<char>(name.size());
memcpy(name_data, name.data(), name.size());
if (mutable_variable) { if (mutable_variable) {
// Allocate a separate variable for the import. // Allocate a separate variable for the import.
DeclareGlobal(info, true, type, vtype); DeclareGlobal(info, true, type, vtype);
// Record the need to initialize the global from the import. // Record the need to initialize the global from the import.
global_imports_.push_back({name_data, name.size(), 0, info->index, true}); global_imports_.push_back({name, 0, info->index, true});
} else { } else {
// Just use the import directly. // Just use the import directly.
global_imports_.push_back({name_data, name.size(), 0, info->index, false}); global_imports_.push_back({name, 0, info->index, false});
} }
GlobalImport& gi = global_imports_.back(); GlobalImport& gi = global_imports_.back();
// TODO(bradnelson): Reuse parse buffer memory / make wasm-module-builder // TODO(bradnelson): Reuse parse buffer memory / make wasm-module-builder
// managed the memory for the import name (currently have to keep our // managed the memory for the import name (currently have to keep our
// own memory for it). // own memory for it).
gi.import_index = module_builder_->AddGlobalImport( gi.import_index = module_builder_->AddGlobalImport(
name_data, static_cast<int>(name.size()), vtype); name.data(), static_cast<int>(name.size()), vtype);
if (!mutable_variable) { if (!mutable_variable) {
info->DeclareGlobalImport(type, gi.import_index); info->DeclareGlobalImport(type, gi.import_index);
} }
...@@ -366,16 +362,6 @@ void AsmJsParser::ValidateModule() { ...@@ -366,16 +362,6 @@ void AsmJsParser::ValidateModule() {
} }
RECURSE(ValidateExport()); RECURSE(ValidateExport());
// Check that all functions were eventually defined.
for (auto info : global_var_info_) {
if (info.kind != VarKind::kFunction) {
continue;
}
if (!info.function_defined) {
FAIL("Undefined function");
}
}
// Add start function to init things. // Add start function to init things.
WasmFunctionBuilder* start = module_builder_->AddFunction(); WasmFunctionBuilder* start = module_builder_->AddFunction();
module_builder_->MarkStartFunction(start); module_builder_->MarkStartFunction(start);
...@@ -461,9 +447,8 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) { ...@@ -461,9 +447,8 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
if (uvalue > 0x7fffffff) { if (uvalue > 0x7fffffff) {
FAIL("Numeric literal out of range"); FAIL("Numeric literal out of range");
} }
DeclareGlobal(info, mutable_variable, DeclareGlobal(info, mutable_variable, AsmType::Int(), kWasmI32,
mutable_variable ? AsmType::Int() : AsmType::Signed(), WasmInitExpr(static_cast<int32_t>(uvalue)));
kWasmI32, WasmInitExpr(static_cast<int32_t>(uvalue)));
} else if (Check('-')) { } else if (Check('-')) {
if (CheckForDouble(&dvalue)) { if (CheckForDouble(&dvalue)) {
DeclareGlobal(info, mutable_variable, AsmType::Double(), kWasmF64, DeclareGlobal(info, mutable_variable, AsmType::Double(), kWasmF64,
...@@ -472,9 +457,8 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) { ...@@ -472,9 +457,8 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
if (uvalue > 0x7fffffff) { if (uvalue > 0x7fffffff) {
FAIL("Numeric literal out of range"); FAIL("Numeric literal out of range");
} }
DeclareGlobal(info, mutable_variable, DeclareGlobal(info, mutable_variable, AsmType::Int(), kWasmI32,
mutable_variable ? AsmType::Int() : AsmType::Signed(), WasmInitExpr(-static_cast<int32_t>(uvalue)));
kWasmI32, WasmInitExpr(-static_cast<int32_t>(uvalue)));
} else { } else {
FAIL("Expected numeric literal"); FAIL("Expected numeric literal");
} }
...@@ -486,33 +470,16 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) { ...@@ -486,33 +470,16 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
} else if (ValidateModuleVarImport(info, mutable_variable)) { } else if (ValidateModuleVarImport(info, mutable_variable)) {
// Handled inside. // Handled inside.
} else if (scanner_.IsGlobal()) { } else if (scanner_.IsGlobal()) {
RECURSE(ValidateModuleVarFromGlobal(info, mutable_variable)); RECURSE(ValidateModuleVarFloat(info, mutable_variable));
} else { } else {
FAIL("Bad variable declaration"); FAIL("Bad variable declaration");
} }
} }
// 6.1 ValidateModule - global float declaration // 6.1 ValidateModule - global float declaration
void AsmJsParser::ValidateModuleVarFromGlobal(VarInfo* info, void AsmJsParser::ValidateModuleVarFloat(VarInfo* info, bool mutable_variable) {
bool mutable_variable) { if (!GetVarInfo(Consume())->type->IsA(stdlib_fround_)) {
VarInfo* src_info = GetVarInfo(Consume()); FAIL("Expected fround");
if (!src_info->type->IsA(stdlib_fround_)) {
if (src_info->mutable_variable) {
FAIL("Can only use immutable variables in global definition");
}
if (mutable_variable) {
FAIL("Can only define immutable variables with other immutables");
}
if (!src_info->type->IsA(AsmType::Int()) &&
!src_info->type->IsA(AsmType::Float()) &&
!src_info->type->IsA(AsmType::Double())) {
FAIL("Expected int, float, double, or fround for global definition");
}
info->kind = VarKind::kGlobal;
info->type = src_info->type;
info->index = src_info->index;
info->mutable_variable = false;
return;
} }
EXPECT_TOKEN('('); EXPECT_TOKEN('(');
bool negate = false; bool negate = false;
...@@ -565,11 +532,7 @@ bool AsmJsParser::ValidateModuleVarImport(VarInfo* info, ...@@ -565,11 +532,7 @@ bool AsmJsParser::ValidateModuleVarImport(VarInfo* info,
info->kind = VarKind::kImportedFunction; info->kind = VarKind::kImportedFunction;
function_import_info_.resize(function_import_info_.size() + 1); function_import_info_.resize(function_import_info_.size() + 1);
info->import = &function_import_info_.back(); info->import = &function_import_info_.back();
// TODO(bradnelson): Refactor memory management here. info->import->name = import_name;
// AsmModuleBuilder should really own import names.
info->import->function_name = zone()->NewArray<char>(import_name.size());
memcpy(info->import->function_name, import_name.data(), import_name.size());
info->import->function_name_size = import_name.size();
return true; return true;
} }
return false; return false;
...@@ -606,7 +569,6 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) { ...@@ -606,7 +569,6 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) {
case TOK(name): \ case TOK(name): \
DeclareGlobal(info, false, AsmType::Double(), kWasmF64, \ DeclareGlobal(info, false, AsmType::Double(), kWasmF64, \
WasmInitExpr(M_##name)); \ WasmInitExpr(M_##name)); \
stdlib_uses_.insert(AsmTyper::kMath##name); \
break; break;
STDLIB_MATH_VALUE_LIST(V) STDLIB_MATH_VALUE_LIST(V)
#undef V #undef V
...@@ -623,11 +585,9 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) { ...@@ -623,11 +585,9 @@ void AsmJsParser::ValidateModuleVarStdlib(VarInfo* info) {
} else if (Check(TOK(Infinity))) { } else if (Check(TOK(Infinity))) {
DeclareGlobal(info, false, AsmType::Double(), kWasmF64, DeclareGlobal(info, false, AsmType::Double(), kWasmF64,
WasmInitExpr(std::numeric_limits<double>::infinity())); WasmInitExpr(std::numeric_limits<double>::infinity()));
stdlib_uses_.insert(AsmTyper::kInfinity);
} else if (Check(TOK(NaN))) { } else if (Check(TOK(NaN))) {
DeclareGlobal(info, false, AsmType::Double(), kWasmF64, DeclareGlobal(info, false, AsmType::Double(), kWasmF64,
WasmInitExpr(std::numeric_limits<double>::quiet_NaN())); WasmInitExpr(std::numeric_limits<double>::quiet_NaN()));
stdlib_uses_.insert(AsmTyper::kNaN);
} else { } else {
FAIL("Invalid member of stdlib"); FAIL("Invalid member of stdlib");
} }
...@@ -735,18 +695,13 @@ void AsmJsParser::ValidateFunction() { ...@@ -735,18 +695,13 @@ void AsmJsParser::ValidateFunction() {
if (function_info->kind == VarKind::kUnused) { if (function_info->kind == VarKind::kUnused) {
function_info->kind = VarKind::kFunction; function_info->kind = VarKind::kFunction;
function_info->function_builder = module_builder_->AddFunction(); function_info->function_builder = module_builder_->AddFunction();
// TODO(bradnelson): Cleanup memory management here.
// WasmModuleBuilder should own these.
char* function_name = zone()->NewArray<char>(function_name_raw.size());
memcpy(function_name, function_name_raw.data(), function_name_raw.size());
function_info->function_builder->SetName( function_info->function_builder->SetName(
{function_name, static_cast<int>(function_name_raw.size())}); {function_name_raw.c_str(),
static_cast<int>(function_name_raw.size())});
function_info->index = function_info->function_builder->func_index(); function_info->index = function_info->function_builder->func_index();
function_info->function_defined = true; function_info->function_defined = true;
} else if (function_info->function_defined) { } else if (function_info->function_defined) {
FAIL("Function redefined"); FAIL("Function redefined");
} else {
function_info->function_defined = true;
} }
current_function_builder_ = function_info->function_builder; current_function_builder_ = function_info->function_builder;
return_type_ = nullptr; return_type_ = nullptr;
...@@ -900,7 +855,7 @@ void AsmJsParser::ValidateFunctionLocals( ...@@ -900,7 +855,7 @@ void AsmJsParser::ValidateFunctionLocals(
info->type = AsmType::Double(); info->type = AsmType::Double();
info->index = static_cast<uint32_t>(param_count + locals->size()); info->index = static_cast<uint32_t>(param_count + locals->size());
locals->push_back(kWasmF64); locals->push_back(kWasmF64);
byte code[] = {WASM_F64(-dvalue)}; byte code[] = {WASM_F64(dvalue)};
current_function_builder_->EmitCode(code, sizeof(code)); current_function_builder_->EmitCode(code, sizeof(code));
current_function_builder_->EmitSetLocal(info->index); current_function_builder_->EmitSetLocal(info->index);
} else if (CheckForUnsigned(&uvalue)) { } else if (CheckForUnsigned(&uvalue)) {
...@@ -2108,9 +2063,8 @@ AsmType* AsmJsParser::ValidateCall() { ...@@ -2108,9 +2063,8 @@ AsmType* AsmJsParser::ValidateCall() {
uint32_t index; uint32_t index;
if (cache_index >= function_info->import->cache_index.size()) { if (cache_index >= function_info->import->cache_index.size()) {
index = module_builder_->AddImport( index = module_builder_->AddImport(
function_info->import->function_name, function_info->import->name.data(),
static_cast<uint32_t>(function_info->import->function_name_size), static_cast<uint32_t>(function_info->import->name.size()), sig);
sig);
function_info->import->cache_index.push_back(index); function_info->import->cache_index.push_back(index);
} else { } else {
index = function_info->import->cache_index[cache_index]; index = function_info->import->cache_index[cache_index];
......
...@@ -59,8 +59,7 @@ class AsmJsParser { ...@@ -59,8 +59,7 @@ class AsmJsParser {
// clang-format on // clang-format on
struct FunctionImportInfo { struct FunctionImportInfo {
char* function_name; std::string name;
size_t function_name_size;
SignatureMap cache; SignatureMap cache;
std::vector<uint32_t> cache_index; std::vector<uint32_t> cache_index;
}; };
...@@ -81,8 +80,7 @@ class AsmJsParser { ...@@ -81,8 +80,7 @@ class AsmJsParser {
}; };
struct GlobalImport { struct GlobalImport {
char* import_name; std::string import_name;
size_t import_name_size;
uint32_t import_index; uint32_t import_index;
uint32_t global_index; uint32_t global_index;
bool needs_init; bool needs_init;
...@@ -250,7 +248,7 @@ class AsmJsParser { ...@@ -250,7 +248,7 @@ class AsmJsParser {
bool ValidateModuleVarImport(VarInfo* info, bool mutable_variable); bool ValidateModuleVarImport(VarInfo* info, bool mutable_variable);
void ValidateModuleVarStdlib(VarInfo* info); void ValidateModuleVarStdlib(VarInfo* info);
void ValidateModuleVarNewStdlib(VarInfo* info); void ValidateModuleVarNewStdlib(VarInfo* info);
void ValidateModuleVarFromGlobal(VarInfo* info, bool mutable_variable); void ValidateModuleVarFloat(VarInfo* info, bool mutable_variable);
void ValidateExport(); // 6.2 ValidateExport void ValidateExport(); // 6.2 ValidateExport
void ValidateFunctionTable(); // 6.3 ValidateFunctionTable void ValidateFunctionTable(); // 6.3 ValidateFunctionTable
......
...@@ -145,14 +145,12 @@ void AsmJsScanner::Next() { ...@@ -145,14 +145,12 @@ void AsmJsScanner::Next() {
} }
void AsmJsScanner::Rewind() { void AsmJsScanner::Rewind() {
// TODO(bradnelson): Currently rewinding needs to leave in place the
// preceding newline state (in case a |0 ends a line).
// This is weird and stateful, fix me.
DCHECK(!rewind_); DCHECK(!rewind_);
next_token_ = token_; next_token_ = token_;
token_ = preceding_token_; token_ = preceding_token_;
preceding_token_ = kUninitialized; preceding_token_ = kUninitialized;
rewind_ = true; rewind_ = true;
preceded_by_newline_ = false;
identifier_string_.clear(); identifier_string_.clear();
} }
......
...@@ -190,10 +190,9 @@ void WasmFunctionBuilder::StashCode(std::vector<byte>* dst, size_t position) { ...@@ -190,10 +190,9 @@ void WasmFunctionBuilder::StashCode(std::vector<byte>* dst, size_t position) {
body_.resize(position); body_.resize(position);
return; return;
} }
DCHECK_LE(position, body_.size());
size_t len = body_.size() - position; size_t len = body_.size() - position;
dst->resize(len); dst->resize(len);
memcpy(dst->data(), body_.data() + position, len); memcpy(dst->data(), &body_[position], len);
body_.resize(position); body_.resize(position);
} }
......
...@@ -339,6 +339,23 @@ function assertValidAsm(func) { ...@@ -339,6 +339,23 @@ function assertValidAsm(func) {
assertFalse(%IsAsmWasmCode(Module)); assertFalse(%IsAsmWasmCode(Module));
})(); })();
(function TestBadishBooleanExprAnnotation() {
function Module() {
"use asm";
function foo(x) {
x = x | 0;
x = (x + 1) | false;
return x | 0;
}
return { foo: foo };
}
var m = Module();
// We all false here because the parser optimizes expressons like:
// !123 to false.
assertTrue(%IsAsmWasmCode(Module));
assertEquals(4, m.foo(3));
})();
(function TestBadFroundTrue() { (function TestBadFroundTrue() {
function Module(stdlib) { function Module(stdlib) {
"use asm"; "use asm";
......
...@@ -662,12 +662,27 @@ ...@@ -662,12 +662,27 @@
['variant == asm_wasm', { ['variant == asm_wasm', {
# Issue 6127: We won't fix these in the "old" validator. But we definitely # Issue 6127: We won't fix these in the "old" validator. But we definitely
# need to re-enable these for the "new" validator. # need to re-enable these for the "new" validator.
'code-coverage-precise': [SKIP],
'object-freeze': [SKIP],
'asm/asm-validation': [SKIP],
'asm/infinite-loops': [SKIP],
'es6/completion': [SKIP],
'es6/function-length-configurable': [SKIP],
'es6/object-assign': [SKIP],
'es6/proxies-cross-realm-exception': [SKIP],
'harmony/function-tostring': [SKIP],
'compiler/regress-445859': [SKIP], 'compiler/regress-445859': [SKIP],
'compiler/regress-452427': [SKIP], 'compiler/regress-452427': [SKIP],
'regress/regress-353058': [SKIP],
'regress/regress-458987': [SKIP], 'regress/regress-458987': [SKIP],
'regress/regress-599068-func-bindings': [SKIP],
'regress/regress-599719': [SKIP], 'regress/regress-599719': [SKIP],
'regress/regress-618608': [SKIP], 'regress/regress-618608': [SKIP],
'regress/regress-619382': [SKIP],
'regress/regress-632289': [SKIP],
'regress/regress-670808': [SKIP], 'regress/regress-670808': [SKIP],
'regress/regress-crbug-530598': [SKIP],
'wasm/asm-wasm': [SKIP],
'wasm/asm-wasm-exception-in-tonumber': [SKIP], 'wasm/asm-wasm-exception-in-tonumber': [SKIP],
'wasm/asm-wasm-stack': [SKIP], 'wasm/asm-wasm-stack': [SKIP],
'wasm/errors': [SKIP], 'wasm/errors': [SKIP],
...@@ -676,6 +691,15 @@ ...@@ -676,6 +691,15 @@
'asm/b5528-comma': [SKIP], 'asm/b5528-comma': [SKIP],
'asm/if-folding': [SKIP], 'asm/if-folding': [SKIP],
'asm/if-reduction': [SKIP], 'asm/if-reduction': [SKIP],
'wasm/embenchen/box2d': [SKIP],
'wasm/embenchen/copy': [SKIP],
'wasm/embenchen/corrections': [SKIP],
'wasm/embenchen/fannkuch': [SKIP],
'wasm/embenchen/fasta': [SKIP],
'wasm/embenchen/lua_binarytrees': [SKIP],
'wasm/embenchen/memops': [SKIP],
'wasm/embenchen/primes': [SKIP],
'wasm/embenchen/zlib': [SKIP],
# Issue 6127: Breaks on no-snap bots. # Issue 6127: Breaks on no-snap bots.
'es8/regress/regress-624300': [SKIP], 'es8/regress/regress-624300': [SKIP],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment