Commit 02d999ab authored by Michael Starzinger's avatar Michael Starzinger Committed by Commit Bot

[asm.js] Fix Math.abs signature to return unsigned.

This fixes the signature of "Math.abs" from "(signed) -> signed" to
"(signed) -> unsigned" and hence fixes cases where the absolute value
would overflow the range of signed 32-bit values. This is in sync with
spec erratas (and ECMAScript semantics).

Note that this also switches the underlying implementation of the above
absolute value function to a branch-free version.

R=clemensh@chromium.org
TEST=mjsunit/regress/regress-6838-3
BUG=v8:6838

Change-Id: Ib13b7ecd336ae386cbde7c574e727bf52f841e00
Reviewed-on: https://chromium-review.googlesource.com/684181
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48169}
parent ab7bd9f4
...@@ -109,8 +109,9 @@ void AsmJsParser::InitializeStdlibTypes() { ...@@ -109,8 +109,9 @@ void AsmJsParser::InitializeStdlibTypes() {
fq2fh->AsFunctionType()->AddArgument(fq); fq2fh->AsFunctionType()->AddArgument(fq);
auto* s = AsmType::Signed(); auto* s = AsmType::Signed();
auto* s2s = AsmType::Function(zone(), s); auto* u = AsmType::Unsigned();
s2s->AsFunctionType()->AddArgument(s); auto* s2u = AsmType::Function(zone(), u);
s2u->AsFunctionType()->AddArgument(s);
auto* i = AsmType::Int(); auto* i = AsmType::Int();
stdlib_i2s_ = AsmType::Function(zone_, s); stdlib_i2s_ = AsmType::Function(zone_, s);
...@@ -135,12 +136,11 @@ void AsmJsParser::InitializeStdlibTypes() { ...@@ -135,12 +136,11 @@ void AsmJsParser::InitializeStdlibTypes() {
// The signatures in "9 Standard Library" of the spec draft are outdated and // The signatures in "9 Standard Library" of the spec draft are outdated and
// have been superseded with the following by an errata: // have been superseded with the following by an errata:
// TODO(mstarzinger): Actually fix Math.abs to return unsigned!
// - Math.abs : (signed) -> unsigned // - Math.abs : (signed) -> unsigned
// (double?) -> double // (double?) -> double
// (float?) -> floatish // (float?) -> floatish
stdlib_abs_ = AsmType::OverloadedFunction(zone()); stdlib_abs_ = AsmType::OverloadedFunction(zone());
stdlib_abs_->AsOverloadedFunctionType()->AddOverload(s2s); stdlib_abs_->AsOverloadedFunctionType()->AddOverload(s2u);
stdlib_abs_->AsOverloadedFunctionType()->AddOverload(stdlib_dq2d_); stdlib_abs_->AsOverloadedFunctionType()->AddOverload(stdlib_dq2d_);
stdlib_abs_->AsOverloadedFunctionType()->AddOverload(fq2fh); stdlib_abs_->AsOverloadedFunctionType()->AddOverload(fq2fh);
...@@ -2223,6 +2223,9 @@ AsmType* AsmJsParser::ValidateCall() { ...@@ -2223,6 +2223,9 @@ AsmType* AsmJsParser::ValidateCall() {
} else if (callable->CanBeInvokedWith(AsmType::Signed(), } else if (callable->CanBeInvokedWith(AsmType::Signed(),
param_specific_types)) { param_specific_types)) {
return_type = AsmType::Signed(); return_type = AsmType::Signed();
} else if (callable->CanBeInvokedWith(AsmType::Unsigned(),
param_specific_types)) {
return_type = AsmType::Unsigned();
} else { } else {
FAILn("Function use doesn't match definition"); FAILn("Function use doesn't match definition");
} }
...@@ -2292,14 +2295,13 @@ AsmType* AsmJsParser::ValidateCall() { ...@@ -2292,14 +2295,13 @@ AsmType* AsmJsParser::ValidateCall() {
if (param_specific_types[0]->IsA(AsmType::Signed())) { if (param_specific_types[0]->IsA(AsmType::Signed())) {
TemporaryVariableScope tmp(this); TemporaryVariableScope tmp(this);
current_function_builder_->EmitTeeLocal(tmp.get()); current_function_builder_->EmitTeeLocal(tmp.get());
current_function_builder_->Emit(kExprI32Clz);
current_function_builder_->EmitWithU8(kExprIf, kLocalI32);
current_function_builder_->EmitGetLocal(tmp.get()); current_function_builder_->EmitGetLocal(tmp.get());
current_function_builder_->Emit(kExprElse); current_function_builder_->EmitI32Const(31);
current_function_builder_->EmitI32Const(0); current_function_builder_->Emit(kExprI32ShrS);
current_function_builder_->EmitTeeLocal(tmp.get());
current_function_builder_->Emit(kExprI32Xor);
current_function_builder_->EmitGetLocal(tmp.get()); current_function_builder_->EmitGetLocal(tmp.get());
current_function_builder_->Emit(kExprI32Sub); current_function_builder_->Emit(kExprI32Sub);
current_function_builder_->Emit(kExprEnd);
} else if (param_specific_types[0]->IsA(AsmType::DoubleQ())) { } else if (param_specific_types[0]->IsA(AsmType::DoubleQ())) {
current_function_builder_->Emit(kExprF64Abs); current_function_builder_->Emit(kExprF64Abs);
} else if (param_specific_types[0]->IsA(AsmType::FloatQ())) { } else if (param_specific_types[0]->IsA(AsmType::FloatQ())) {
......
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
(function TestMathAbsReturningUnsigned() {
function Module(stdlib) {
"use asm";
var abs=stdlib.Math.abs;
function f(a, b) {
a = a | 0;
b = b | 0;
return (abs(a >> 0) == (b >>> 0)) | 0;
}
return f;
}
var f = Module(this);
assertEquals(0, f(1, 2));
assertEquals(1, f(23, 23));
assertEquals(1, f(-42, 42));
assertEquals(1, f(-0x7fffffff, 0x7fffffff));
assertEquals(1, f(-0x80000000, 0x80000000));
assertTrue(%IsAsmWasmCode(Module));
})();
(function TestMathAbsOverflowSignedValue() {
function Module(stdlib) {
"use asm";
var abs=stdlib.Math.abs;
function f() {
return (abs(-0x80000000) > 0) | 0;
}
return f;
}
var f = Module(this);
assertEquals(1, f());
assertTrue(%IsAsmWasmCode(Module));
})();
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