-
Notifications
You must be signed in to change notification settings - Fork 94
WIP: Transpile sign extension proposal #375
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
a77d670
fb5bd7b
8143ee6
1872d94
4930ae8
a23af0f
4a27ad5
258091f
fe73f49
75cf616
c7b349d
2c07ba0
fb971f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Sign extension proposal | ||
|
||
<https://github.com/WebAssembly/sign-extension-ops> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
{ | ||
"name": "@webassemblyjs/proposal-sign-ops", | ||
"version": "1.5.6", | ||
"description": "", | ||
"main": "lib/index.js", | ||
"scripts": { | ||
"test": "echo \"Error: no test specified\" && exit 1" | ||
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/xtuc/webassemblyjs.git" | ||
}, | ||
"publishConfig": { | ||
"access": "public" | ||
}, | ||
"author": "Mauro Bringolf", | ||
"license": "MIT", | ||
"dependencies": { | ||
"@webassemblyjs/ast": "1.5.6", | ||
"@webassemblyjs/helper-buffer": "1.5.6", | ||
"@webassemblyjs/helper-wasm-bytecode": "1.5.6", | ||
"@webassemblyjs/helper-wasm-section": "1.5.6", | ||
"@webassemblyjs/wasm-edit": "1.5.6", | ||
"@webassemblyjs/wasm-gen": "1.5.6", | ||
"@webassemblyjs/wasm-opt": "1.5.6", | ||
"debug": "^3.1.0" | ||
}, | ||
"devDependencies": { | ||
"@webassemblyjs/helper-test-framework": "1.5.6", | ||
"@webassemblyjs/wast-parser": "1.5.6", | ||
"@webassemblyjs/wasm-parser": "1.5.6", | ||
"@webassemblyjs/wast-printer": "1.5.6", | ||
"wabt": "1.0.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
import { traverse, callInstruction, numberLiteral } from "@webassemblyjs/ast"; | ||
import { parse } from "@webassemblyjs/wast-parser"; | ||
import { readFileSync } from "fs"; | ||
import path from "path"; | ||
|
||
const funcFromWast = sourcePath => { | ||
const ast = parse(readFileSync(sourcePath, "utf8")); | ||
return ast.body[0].fields.find(f => f.type === "Func"); | ||
}; | ||
|
||
class Polyfills { | ||
constructor(startIndex) { | ||
this.startIndex = startIndex; | ||
|
||
this.asts = { | ||
i32_extend8_s: funcFromWast( | ||
path.join(__dirname, "/polyfills/i32_extend8_s.wast") | ||
), | ||
i32_extend16_s: funcFromWast( | ||
path.join(__dirname, "/polyfills/i32_extend16_s.wast") | ||
), | ||
i64_extend8_s: funcFromWast( | ||
path.join(__dirname, "/polyfills/i64_extend8_s.wast") | ||
), | ||
i64_extend16_s: funcFromWast( | ||
path.join(__dirname, "/polyfills/i64_extend16_s.wast") | ||
), | ||
i64_extend32_s: funcFromWast( | ||
path.join(__dirname, "/polyfills/i64_extend32_s.wast") | ||
) | ||
}; | ||
|
||
this.instructions = Object.keys(this.asts); | ||
|
||
this.index = {}; | ||
|
||
this.instructions.forEach(instrName => { | ||
this.index[instrName] = -1; | ||
}); | ||
} | ||
|
||
replaceWith(path, instrName) { | ||
if (this.index[instrName] === -1) { | ||
this.index[instrName] = this.startIndex++; | ||
} | ||
|
||
const index = this.index[instrName]; | ||
path.replaceWith(callInstruction(numberLiteral(index, String(index)))); | ||
} | ||
|
||
matchesInstruction(instrName) { | ||
return this.instructions.includes(instrName); | ||
} | ||
|
||
insertInto(ast) { | ||
const funcsToPush = Object.keys(this.index) | ||
.filter(instrName => this.index[instrName] >= 0) | ||
.sort((x, y) => (this.index[x] > this.index[y] ? 1 : -1)) | ||
.map(instrName => this.asts[instrName]); | ||
|
||
ast.body[0].fields.push(...funcsToPush); | ||
} | ||
} | ||
|
||
export function transformAst(ast) { | ||
let numberOfFuncs = 0; | ||
|
||
const countFuncVisitor = { | ||
Func() { | ||
++numberOfFuncs; | ||
} | ||
}; | ||
|
||
traverse(ast, countFuncVisitor); | ||
|
||
const polyfills = new Polyfills(numberOfFuncs); | ||
|
||
const signExtendVisitor = { | ||
Instr(path) { | ||
if (polyfills.matchesInstruction(path.node.id)) { | ||
polyfills.replaceWith(path, path.node.id); | ||
} | ||
} | ||
}; | ||
|
||
traverse(ast, signExtendVisitor); | ||
|
||
polyfills.insertInto(ast); | ||
|
||
return ast; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
(module | ||
(type (func (param i32) (result i32))) | ||
(func $i32_extend16_s (param i32) (result i32) | ||
(get_local 0) | ||
(i32.const -32768) | ||
(i32.or) | ||
(get_local 0) | ||
(i32.const 32767) | ||
(i32.and) | ||
(get_local 0) | ||
(i32.const 32768) | ||
(i32.and) | ||
(select) | ||
) | ||
(export "i32_extend16_s" (func $i32_extend16_s)) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
(module | ||
(type (func (param i32) (result i32))) | ||
(func $i32_extend8_s (param i32) (result i32) | ||
(get_local 0) | ||
(i32.const -128) | ||
(i32.or) | ||
(get_local 0) | ||
(i32.const 127) | ||
(i32.and) | ||
(get_local 0) | ||
(i32.const 128) | ||
(i32.and) | ||
(select) | ||
) | ||
(export "i32_extend8_s" (func $i32_extend8_s)) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
(module | ||
(type (func (param i64) (result i64))) | ||
(func $i64_extend16_s (param i64) (result i64) | ||
(get_local 0) | ||
(i64.const -32768) | ||
(i64.or) | ||
(get_local 0) | ||
(i64.const 32767) | ||
(i64.and) | ||
(get_local 0) | ||
(i64.const 32768) | ||
(i64.and) | ||
(i64.const 0) | ||
(i64.ne) | ||
(select) | ||
) | ||
(export "i64_extend16_s" (func $i64_extend16_s)) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
(module | ||
(type (func (param i64) (result i64))) | ||
(func $i64_extend32_s (param i64) (result i64) | ||
(get_local 0) | ||
(i64.const -2147483648) | ||
(i64.or) | ||
(get_local 0) | ||
(i64.const 2147483647) | ||
(i64.and) | ||
(get_local 0) | ||
(i64.const 2147483648) | ||
(i64.and) | ||
(i64.const 0) | ||
(i64.ne) | ||
(select) | ||
) | ||
(export "i64_extend32_s" (func $i64_extend32_s)) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
(module | ||
(type (func (param i64) (result i64))) | ||
(func $i64_extend8_s (param i64) (result i64) | ||
(get_local 0) | ||
(i64.const -128) | ||
(i64.or) | ||
(get_local 0) | ||
(i64.const 127) | ||
(i64.and) | ||
(get_local 0) | ||
(i64.const 128) | ||
(i64.and) | ||
(i64.const 0) | ||
(i64.ne) | ||
(select) | ||
) | ||
(export "i64_extend8_s" (func $i64_extend8_s)) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
const path = require("path"); | ||
const wabt = require("wabt"); | ||
const assert = require("assert"); | ||
|
||
const { readFileSync } = require("fs"); | ||
|
||
const polyfills = [ | ||
{ | ||
name: "i32_extend8_s", | ||
fixtures: [ | ||
[0, 0], | ||
[0x7f, 127], | ||
[0x80, -128], | ||
[0xff, -1], | ||
[0x01234500, 0], | ||
[0xfedcba80, -0x80], | ||
[-1, -1] | ||
] | ||
}, | ||
{ | ||
name: "i32_extend16_s", | ||
fixtures: [ | ||
[0, 0], | ||
[0x7fff, 32767], | ||
[0x8000, -32768], | ||
[0xffff, -1], | ||
[0x01230000, 0], | ||
[0xfedc8000, -0x8000], | ||
[-1, -1] | ||
] | ||
} | ||
]; | ||
|
||
polyfills.forEach(p => { | ||
p.path = path.join(__dirname, `../lib/polyfills/${p.name}.wast`); | ||
}); | ||
|
||
polyfills.forEach(polyfill => { | ||
describe(polyfill.path, () => { | ||
const wast = readFileSync(polyfill.path, "utf8").trim(); | ||
const wasm = wabt | ||
.parseWat(polyfill.path, wast) | ||
.toBinary({ write_debug_names: false }); | ||
|
||
it("should return correct values", () => { | ||
return WebAssembly.instantiate(wasm.buffer).then(result => { | ||
const polyfillFn = result.instance.exports[polyfill.name]; | ||
polyfill.fixtures.forEach(([input, output]) => { | ||
assert.equal(output, polyfillFn(input)); | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
const { parse } = require("@webassemblyjs/wast-parser"); | ||
const { print } = require("@webassemblyjs/wast-printer"); | ||
const path = require("path"); | ||
|
||
const { | ||
getFixtures, | ||
compareStrings | ||
} = require("@webassemblyjs/helper-test-framework"); | ||
const { readFileSync, existsSync, writeFileSync } = require("fs"); | ||
|
||
const { transformAst } = require("../lib/"); | ||
|
||
const testCases = getFixtures( | ||
path.join(__dirname, "./wast/"), | ||
"/**/input.wast" | ||
); | ||
|
||
testCases.forEach(testCase => { | ||
describe(testCase, () => { | ||
const input = readFileSync(testCase, "utf8").trim(); | ||
|
||
it("should transpile sign extension operators into functions correctly", () => { | ||
const actualOutput = print(transformAst(parse(input))); | ||
|
||
if (!existsSync(path.join(path.dirname(testCase), "output.wast"))) { | ||
writeFileSync( | ||
path.join(path.dirname(testCase), "output.wast"), | ||
actualOutput | ||
); | ||
return; | ||
} | ||
|
||
const expectedOutput = readFileSync( | ||
path.join(path.dirname(testCase), "output.wast"), | ||
"utf8" | ||
).trim(); | ||
|
||
compareStrings(actualOutput, expectedOutput); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
(module | ||
(func) | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
(module | ||
(func) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this test is very useful There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah this was my test to check that the test setup works before having the transform. I dont think it is useless though, empty function body might trigger an edge case somewhere. But we might also remove it |
||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
(module | ||
(func (param i32) (result i32) | ||
(get_local 0) | ||
(i32_extend16_s) | ||
(i32_extend8_s) | ||
) | ||
(func (param i32) (result i32) | ||
(get_local 0) | ||
) | ||
(func (param i32) (result i32) | ||
(get_local 0) | ||
(i32_extend8_s) | ||
) | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should change that to support two or more bytes? Atomic operations also have an additional byte.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to the threads proposal? i.e. https://webassembly.github.io/threads/binary/instructions.html#atomic-memory-instructions. Then it shouldn't affect this proposal but something to look out for in the future I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already added a check for the atomic operations prefix byte. I just saying that we could move the logic here by matching an array of bytes?