Skip to content

Add support for sign-extension operators from threading proposal #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dschuff merged 2 commits into from
Sep 6, 2017
Merged

Add support for sign-extension operators from threading proposal #1167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6b97ca
Add support for sign-extension operators from threading proposal
dschuff Sep 6, 2017
466f19c
review comments
dschuff Sep 6, 2017
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
2 changes: 2 additions & 0 deletions src/passes/OptimizeInstructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include <ast/properties.h>
#include <ast/literal-utils.h>

// TODO: Use the new sign-extension opcodes where appropriate. This needs to be conditionalized on the availability of atomics.

namespace wasm {

Name I32_EXPR = "i32.expr",
Expand Down
5 changes: 5 additions & 0 deletions src/passes/Print.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,11 @@ struct PrintSExpression : public Visitor<PrintSExpression> {
case DemoteFloat64: o << "f32.demote/f64"; break;
case ReinterpretInt32: o << "f32.reinterpret/i32"; break;
case ReinterpretInt64: o << "f64.reinterpret/i64"; break;
case ExtendS8Int32: o << "i32.extend8_s"; break;
case ExtendS16Int32: o << "i32.extend16_s"; break;
case ExtendS8Int64: o << "i64.extend8_s"; break;
case ExtendS16Int64: o << "i64.extend16_s"; break;
case ExtendS32Int64: o << "i64.extend32_s"; break;
default: abort();
}
incIndent();
Expand Down
6 changes: 6 additions & 0 deletions src/wasm-binary.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,12 @@ enum ASTNodes {
F32ReinterpretI32 = 0xbe,
F64ReinterpretI64 = 0xbf,

I32ExtendS8 = 0xc0,
I32ExtendS16 = 0xc1,
I64ExtendS8 = 0xc2,
I64ExtendS16 = 0xc3,
I64ExtendS32 = 0xc4,

AtomicPrefix = 0xfe
};

Expand Down
4 changes: 4 additions & 0 deletions src/wasm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ enum UnaryOp {
PromoteFloat32, // f32 to f64
DemoteFloat64, // f64 to f32
ReinterpretInt32, ReinterpretInt64, // reinterpret bits to float
// The following sign-extention operators go along with wasm atomics support.
// Extend signed subword-sized integer. This differs from e.g. ExtendSInt32
// because the input integer is in an i64 value insetad of an i32 value.
ExtendS8Int32, ExtendS16Int32, ExtendS8Int64, ExtendS16Int64, ExtendS32Int64,
};

enum BinaryOp {
Expand Down
11 changes: 11 additions & 0 deletions src/wasm/wasm-binary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,11 @@ void WasmBinaryWriter::visitUnary(Unary *curr) {
case ReinterpretFloat64: o << int8_t(BinaryConsts::I64ReinterpretF64); break;
case ReinterpretInt32: o << int8_t(BinaryConsts::F32ReinterpretI32); break;
case ReinterpretInt64: o << int8_t(BinaryConsts::F64ReinterpretI64); break;
case ExtendS8Int32: o << int8_t(BinaryConsts::I32ExtendS8); break;
case ExtendS16Int32: o << int8_t(BinaryConsts::I32ExtendS16); break;
case ExtendS8Int64: o << int8_t(BinaryConsts::I64ExtendS8); break;
case ExtendS16Int64: o << int8_t(BinaryConsts::I64ExtendS16); break;
case ExtendS32Int64: o << int8_t(BinaryConsts::I64ExtendS32); break;
default: abort();
}
if (curr->type == unreachable) {
Expand Down Expand Up @@ -2679,6 +2684,12 @@ bool WasmBinaryBuilder::maybeVisitUnary(Expression*& out, uint8_t code) {
case BinaryConsts::F32ReinterpretI32: curr = allocator.alloc<Unary>(); curr->op = ReinterpretInt32; curr->type = f32; break;
case BinaryConsts::F64ReinterpretI64: curr = allocator.alloc<Unary>(); curr->op = ReinterpretInt64; curr->type = f64; break;

case BinaryConsts::I32ExtendS8: curr = allocator.alloc<Unary>(); curr->op = ExtendS8Int32; curr->type = i32; break;
case BinaryConsts::I32ExtendS16: curr = allocator.alloc<Unary>(); curr->op = ExtendS16Int32; curr->type = i32; break;
case BinaryConsts::I64ExtendS8: curr = allocator.alloc<Unary>(); curr->op = ExtendS8Int64; curr->type = i64; break;
case BinaryConsts::I64ExtendS16: curr = allocator.alloc<Unary>(); curr->op = ExtendS16Int64; curr->type = i64; break;
case BinaryConsts::I64ExtendS32: curr = allocator.alloc<Unary>(); curr->op = ExtendS32Int64; curr->type = i64; break;

default: return false;
}
if (debug) std::cerr << "zz node: Unary" << std::endl;
Expand Down
9 changes: 8 additions & 1 deletion src/wasm/wasm-s-parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,12 @@ Expression* SExpressionWasmBuilder::makeExpression(Element& s) {
if (op[2] == 0) return makeBinary(s, BINARY_INT_OR_FLOAT(Eq), type);
if (op[2] == 'z') return makeUnary(s, type == i32 ? UnaryOp::EqZInt32 : UnaryOp::EqZInt64, type);
}
if (op[1] == 'x') return makeUnary(s, op[7] == 'u' ? UnaryOp::ExtendUInt32 : UnaryOp::ExtendSInt32, type);
if (op[1] == 'x') {
if (op[6] == '8') return makeUnary(s, type == i32 ? UnaryOp::ExtendS8Int32 : UnaryOp::ExtendS8Int64, type);
if (op[6] == '1') return makeUnary(s, type == i32 ? UnaryOp::ExtendS16Int32 : UnaryOp::ExtendS16Int64, type);
if (op[6] == '3') return makeUnary(s, UnaryOp::ExtendS32Int64, type);
return makeUnary(s, op[7] == 'u' ? UnaryOp::ExtendUInt32 : UnaryOp::ExtendSInt32, type);
}
abort_on(op);
}
case 'f': {
Expand Down Expand Up @@ -920,6 +925,8 @@ Expression* SExpressionWasmBuilder::makeUnary(Element& s, UnaryOp op, WasmType t
break;
}
case ExtendSInt32: case ExtendUInt32:
case ExtendS8Int32: case ExtendS16Int32:
case ExtendS8Int64: case ExtendS16Int64: case ExtendS32Int64:
case WrapInt64:
case PromoteFloat32:
case DemoteFloat64:
Expand Down
13 changes: 11 additions & 2 deletions src/wasm/wasm-validator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,17 @@ void WasmValidator::visitUnary(Unary *curr) {
shouldBeTrue(curr->value->type == i64, curr, "i64.eqz input must be i64");
break;
}
case ExtendSInt32: shouldBeEqual(curr->value->type, i32, curr, "extend type must be correct"); break;
case ExtendUInt32: shouldBeEqual(curr->value->type, i32, curr, "extend type must be correct"); break;
case ExtendSInt32:
Copy link
Member

@binji binji Sep 6, 2017

Choose a reason for hiding this comment

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

Perhaps the original extend instructions should be renamed now?

Copy link
Member Author

@dschuff dschuff Sep 6, 2017

Choose a reason for hiding this comment

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

Maybe. Actually a lot of instructions probably should be renamed, and probably along with the binary format opcodes, it's kind of a mess (a lot of the opcode names predate the current spec scheme). I almost renamed the extension instructions here but decided that's not a great idea to mix it in with a functionality PR.

Copy link
Member

@binji binji Sep 6, 2017

Choose a reason for hiding this comment

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

Makes sense, probably better for a cleanup PR.

Copy link
Member

@kripken kripken Sep 6, 2017

Choose a reason for hiding this comment

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

Agreed, let's do a bulk renaming eventually to the spec names, but not here.

case ExtendUInt32:
case ExtendS8Int32:
case ExtendS16Int32: {
shouldBeEqual(curr->value->type, i32, curr, "extend type must be correct"); break;
}
case ExtendS8Int64:
case ExtendS16Int64:
case ExtendS32Int64: {
shouldBeEqual(curr->value->type, i64, curr, "extend type must be correct"); break;
}
case WrapInt64: shouldBeEqual(curr->value->type, i64, curr, "wrap type must be correct"); break;
case TruncSFloat32ToInt32: shouldBeEqual(curr->value->type, f32, curr, "trunc type must be correct"); break;
case TruncSFloat32ToInt64: shouldBeEqual(curr->value->type, f32, curr, "trunc type must be correct"); break;
Expand Down
3 changes: 2 additions & 1 deletion src/wasm/wasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ void Unary::finalize() {
case SqrtFloat64: type = value->type; break;
case EqZInt32:
case EqZInt64: type = i32; break;
case ExtendSInt32: case ExtendUInt32: type = i64; break;
case ExtendS8Int32: case ExtendS16Int32: type = i32; break;
case ExtendSInt32: case ExtendUInt32: case ExtendS8Int64: case ExtendS16Int64: case ExtendS32Int64: type = i64; break;
case WrapInt64: type = i32; break;
case PromoteFloat32: type = f64; break;
case DemoteFloat64: type = f32; break;
Expand Down
12 changes: 12 additions & 0 deletions test/signext.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
(module
(type $0 (func))
(func $signext (type $0)
(local $0 i32)
(local $1 i64)
(drop (i32.extend8_s (get_local $0)))
(drop (i32.extend16_s (get_local $0)))
(drop (i64.extend8_s (get_local $1)))
(drop (i64.extend16_s (get_local $1)))
(drop (i64.extend32_s (get_local $1)))
)
)
33 changes: 33 additions & 0 deletions test/signext.wast.from-wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
(module
(type $0 (func))
(memory $0 0)
(func $signext (type $0)
(local $0 i32)
(local $1 i64)
(drop
(i32.extend8_s
(get_local $0)
)
)
(drop
(i32.extend16_s
(get_local $0)
)
)
(drop
(i64.extend8_s
(get_local $1)
)
)
(drop
(i64.extend16_s
(get_local $1)
)
)
(drop
(i64.extend32_s
(get_local $1)
)
)
)
)
36 changes: 36 additions & 0 deletions test/signext.wast.fromBinary
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
(module
(type $0 (func))
(memory $0 0)
(func $signext (type $0)
(local $var$0 i32)
(local $var$1 i64)
(block $label$0
(drop
(i32.extend8_s
(get_local $var$0)
)
)
(drop
(i32.extend16_s
(get_local $var$0)
)
)
(drop
(i64.extend8_s
(get_local $var$1)
)
)
(drop
(i64.extend16_s
(get_local $var$1)
)
)
(drop
(i64.extend32_s
(get_local $var$1)
)
)
)
)
)

36 changes: 36 additions & 0 deletions test/signext.wast.fromBinary.noDebugInfo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
(module
(type $0 (func))
(memory $0 0)
(func $0 (type $0)
(local $var$0 i32)
(local $var$1 i64)
(block $label$0
(drop
(i32.extend8_s
(get_local $var$0)
)
)
(drop
(i32.extend16_s
(get_local $var$0)
)
)
(drop
(i64.extend8_s
(get_local $var$1)
)
)
(drop
(i64.extend16_s
(get_local $var$1)
)
)
(drop
(i64.extend32_s
(get_local $var$1)
)
)
)
)
)

2 changes: 1 addition & 1 deletion test/spec
Open in desktop
Submodule spec updated 2 files
+1 −1 return.wast
+1 −1 unreachable.wast