Implement the signed division instruction in WebAssembly
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Sep 2015 22:47:23 +0000 (22:47 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Sep 2015 22:47:23 +0000 (22:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148772

Patch by Sukolsak Sakshuwong <sukolsak@gmail.com> on 2015-09-04
Reviewed by Geoffrey Garen.

This patch implements the signed division instruction in WebAssembly
for 32-bit integers. We use the IDIV instruction on x86 and x86-64 and
use a C function on all other platforms. We throw an exception if
- the denominator is zero, or
- the numerator is -2^31 and the denominator is -1.

* jit/JITOperations.cpp:
* jit/JITOperations.h:
* tests/stress/wasm-arithmetic.js:
(shouldBe):
(shouldThrow):
* tests/stress/wasm-arithmetic.wasm:
* wasm/WASMFunctionCompiler.h:
(JSC::operationDiv):
(JSC::WASMFunctionCompiler::endFunction):
(JSC::WASMFunctionCompiler::buildBinaryI32):
(JSC::WASMFunctionCompiler::appendCall):
(JSC::WASMFunctionCompiler::appendCallWithExceptionCheck):
(JSC::WASMFunctionCompiler::callOperation):
(JSC::WASMFunctionCompiler::throwStackOverflowError): Deleted.
* wasm/WASMFunctionParser.cpp:
(JSC::WASMFunctionParser::parseExpressionI32):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189396 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/JITOperations.h
Source/JavaScriptCore/tests/stress/wasm-arithmetic.js
Source/JavaScriptCore/tests/stress/wasm-arithmetic.wasm
Source/JavaScriptCore/wasm/WASMFunctionCompiler.h
Source/JavaScriptCore/wasm/WASMFunctionParser.cpp

index a5ebebb..723f19a 100644 (file)
@@ -1,5 +1,35 @@
 2015-09-04  Sukolsak Sakshuwong  <sukolsak@gmail.com>
 
+        Implement the signed division instruction in WebAssembly
+        https://bugs.webkit.org/show_bug.cgi?id=148772
+
+        Reviewed by Geoffrey Garen.
+
+        This patch implements the signed division instruction in WebAssembly
+        for 32-bit integers. We use the IDIV instruction on x86 and x86-64 and
+        use a C function on all other platforms. We throw an exception if
+        - the denominator is zero, or
+        - the numerator is -2^31 and the denominator is -1.
+
+        * jit/JITOperations.cpp:
+        * jit/JITOperations.h:
+        * tests/stress/wasm-arithmetic.js:
+        (shouldBe):
+        (shouldThrow):
+        * tests/stress/wasm-arithmetic.wasm:
+        * wasm/WASMFunctionCompiler.h:
+        (JSC::operationDiv):
+        (JSC::WASMFunctionCompiler::endFunction):
+        (JSC::WASMFunctionCompiler::buildBinaryI32):
+        (JSC::WASMFunctionCompiler::appendCall):
+        (JSC::WASMFunctionCompiler::appendCallWithExceptionCheck):
+        (JSC::WASMFunctionCompiler::callOperation):
+        (JSC::WASMFunctionCompiler::throwStackOverflowError): Deleted.
+        * wasm/WASMFunctionParser.cpp:
+        (JSC::WASMFunctionParser::parseExpressionI32):
+
+2015-09-04  Sukolsak Sakshuwong  <sukolsak@gmail.com>
+
         Implement the GetLocal and SetLocal instructions in WebAssembly
         https://bugs.webkit.org/show_bug.cgi?id=148793
 
index 8c6bee6..be3a9b0 100644 (file)
@@ -95,6 +95,19 @@ void JIT_OPERATION operationThrowStackOverflowError(ExecState* exec, CodeBlock*
     vm->throwException(callerFrame, createStackOverflowError(callerFrame));
 }
 
+#if ENABLE(WEBASSEMBLY)
+void JIT_OPERATION operationThrowDivideError(ExecState* exec)
+{
+    VM* vm = &exec->vm();
+    VMEntryFrame* vmEntryFrame = vm->topVMEntryFrame;
+    CallFrame* callerFrame = exec->callerFrame(vmEntryFrame);
+
+    NativeCallFrameTracerWithRestore tracer(vm, vmEntryFrame, callerFrame);
+    ErrorHandlingScope errorScope(*vm);
+    vm->throwException(callerFrame, createError(callerFrame, ASCIILiteral("Division by zero or division overflow.")));
+}
+#endif
+
 int32_t JIT_OPERATION operationCallArityCheck(ExecState* exec)
 {
     VM* vm = &exec->vm();
index ccc5126..b559a83 100644 (file)
@@ -245,6 +245,9 @@ void JIT_OPERATION lookupExceptionHandlerFromCallerFrame(VM*, ExecState*) WTF_IN
 void JIT_OPERATION operationVMHandleException(ExecState*) WTF_INTERNAL;
 
 void JIT_OPERATION operationThrowStackOverflowError(ExecState*, CodeBlock*) WTF_INTERNAL;
+#if ENABLE(WEBASSEMBLY)
+void JIT_OPERATION operationThrowDivideError(ExecState*) WTF_INTERNAL;
+#endif
 int32_t JIT_OPERATION operationCallArityCheck(ExecState*) WTF_INTERNAL;
 int32_t JIT_OPERATION operationConstructArityCheck(ExecState*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationGetById(ExecState*, StructureStubInfo*, EncodedJSValue, UniquedStringImpl*) WTF_INTERNAL;
index b37a3cc..4a42996 100644 (file)
@@ -1,22 +1,71 @@
 //@ skip
 
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function shouldThrow(func, message) {
+    var error = null;
+    try {
+        func();
+    } catch (e) {
+        error = e;
+    }
+    if (!error)
+        throw new Error("not thrown.");
+    if (String(error) !== message)
+        throw new Error("bad error: " + String(error));
+}
+
 /*
 wasm-arithmetic.wasm is generated by pack-asmjs <https://github.com/WebAssembly/polyfill-prototype-1> from the following script:
 
 function asmModule(global, env, buffer) {
     "use asm";
 
-    function main() {
-        return (10 + 40) - 8;
+    function addSubtract() {
+        return ((10 + 40) - 8) | 0;
+    }
+
+    function addOverflow() {
+        return (2147483647 + 1) | 0;
+    }
+
+    function divide() {
+        return (42 / 5) | 0;
+    }
+
+    function divideByZero() {
+        return (1 / 0) | 0;
+    }
+
+    function divideOverflow() {
+        return (-2147483648 / -1) | 0;
     }
 
     return {
-        main: main
+        addSubtract: addSubtract,
+        addOverflow: addOverflow,
+        divide: divide,
+        divideByZero: divideByZero,
+        divideOverflow: divideOverflow,
     };
 }
 */
 
 var module = loadWebAssembly("wasm-arithmetic.wasm");
-var result = module.main();
-if (result != 42)
-    throw "Error: bad result: " + result;
+
+shouldBe(module.addSubtract(), 42);
+
+shouldBe(module.addOverflow(), -2147483648);
+
+shouldBe(module.divide(), 8);
+
+shouldThrow(() => {
+    module.divideByZero();
+}, "Error: Division by zero or division overflow.");
+
+shouldThrow(() => {
+    module.divideOverflow();
+}, "Error: Division by zero or division overflow.");
index 34ed85d..4ccd7b6 100644 (file)
Binary files a/Source/JavaScriptCore/tests/stress/wasm-arithmetic.wasm and b/Source/JavaScriptCore/tests/stress/wasm-arithmetic.wasm differ
index f00f0d1..99d1490 100644 (file)
 
 namespace JSC {
 
+#if !CPU(X86) && !CPU(X86_64)
+static int32_t JIT_OPERATION operationDiv(int32_t left, int32_t right)
+{
+    return left / right;
+}
+#endif
+
 class WASMFunctionCompiler : private CCallHelpers {
 public:
     typedef int Expression;
@@ -108,7 +115,8 @@ public:
         m_stackOverflow.link(this);
         if (maxFrameExtentForSlowPathCall)
             addPtr(TrustedImm32(-maxFrameExtentForSlowPathCall), stackPointerRegister);
-        throwStackOverflowError();
+        setupArgumentsWithExecState(TrustedImmPtr(m_codeBlock));
+        appendCallWithExceptionCheck(operationThrowStackOverflowError);
 
         // FIXME: Implement arity check.
         Label arityCheck = label();
@@ -116,6 +124,31 @@ public:
         emitPutImmediateToCallFrameHeader(m_codeBlock, JSStack::CodeBlock);
         jump(m_beginLabel);
 
+        if (!m_divideErrorJumpList.empty()) {
+            m_divideErrorJumpList.link(this);
+
+            if (maxFrameExtentForSlowPathCall)
+                addPtr(TrustedImm32(-maxFrameExtentForSlowPathCall), stackPointerRegister);
+            setupArgumentsExecState();
+            appendCallWithExceptionCheck(operationThrowDivideError);
+        }
+
+        if (!m_exceptionChecks.empty()) {
+            m_exceptionChecks.link(this);
+
+            // lookupExceptionHandler is passed two arguments, the VM and the exec (the CallFrame*).
+            move(TrustedImmPtr(vm()), GPRInfo::argumentGPR0);
+            move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
+
+#if CPU(X86)
+            // FIXME: should use the call abstraction, but this is currently in the SpeculativeJIT layer!
+            poke(GPRInfo::argumentGPR0);
+            poke(GPRInfo::argumentGPR1, 1);
+#endif
+            m_calls.append(std::make_pair(call(), FunctionPtr(lookupExceptionHandlerFromCallerFrame).value()));
+            jumpToExceptionHandler();
+        }
+
         LinkBuffer patchBuffer(*m_vm, *this, m_codeBlock, JITCompilationMustSucceed);
 
         for (auto iterator : m_calls)
@@ -189,6 +222,25 @@ public:
         case WASMOpExpressionI32::Sub:
             sub32(GPRInfo::regT1, GPRInfo::regT0);
             break;
+        case WASMOpExpressionI32::SDiv: {
+            m_divideErrorJumpList.append(branchTest32(Zero, GPRInfo::regT1));
+            Jump denominatorNotNeg1 = branch32(NotEqual, GPRInfo::regT1, TrustedImm32(-1));
+            m_divideErrorJumpList.append(branch32(Equal, GPRInfo::regT0, TrustedImm32(-2147483647-1)));
+            denominatorNotNeg1.link(this);
+#if CPU(X86) || CPU(X86_64)
+            ASSERT(GPRInfo::regT0 == X86Registers::eax);
+            move(GPRInfo::regT1, X86Registers::ecx);
+            m_assembler.cdq();
+            m_assembler.idivl_r(X86Registers::ecx);
+#else
+            if (maxFrameExtentForSlowPathCall)
+                addPtr(TrustedImm32(-maxFrameExtentForSlowPathCall), stackPointerRegister);
+            callOperation(operationDiv, GPRInfo::regT0, GPRInfo::regT1, GPRInfo::regT0);
+            if (maxFrameExtentForSlowPathCall)
+                addPtr(TrustedImm32(maxFrameExtentForSlowPathCall), stackPointerRegister);
+#endif
+            break;
+        }
         default:
             ASSERT_NOT_REACHED();
         }
@@ -216,22 +268,22 @@ private:
         return Address(GPRInfo::callFrameRegister, -(m_numberOfLocals + temporaryIndex + 1) * sizeof(StackSlot));
     }
 
-    void throwStackOverflowError()
+    void appendCall(const FunctionPtr& function)
     {
-        setupArgumentsWithExecState(TrustedImmPtr(m_codeBlock));
+        m_calls.append(std::make_pair(call(), function.value()));
+    }
 
-        m_calls.append(std::make_pair(call(), FunctionPtr(operationThrowStackOverflowError).value()));
+    void appendCallWithExceptionCheck(const FunctionPtr& function)
+    {
+        appendCall(function);
+        m_exceptionChecks.append(emitExceptionCheck());
+    }
 
-        // lookupExceptionHandlerFromCallerFrame is passed two arguments, the VM and the exec (the CallFrame*).
-        move(TrustedImmPtr(m_vm), GPRInfo::argumentGPR0);
-        move(GPRInfo::callFrameRegister, GPRInfo::argumentGPR1);
-#if CPU(X86)
-        // FIXME: should use the call abstraction, but this is currently in the SpeculativeJIT layer!
-        poke(GPRInfo::argumentGPR0);
-        poke(GPRInfo::argumentGPR1, 1);
-#endif
-        m_calls.append(std::make_pair(call(), FunctionPtr(lookupExceptionHandlerFromCallerFrame).value()));
-        jumpToExceptionHandler();
+    void callOperation(int32_t JIT_OPERATION (*operation)(int32_t, int32_t), GPRReg src1, GPRReg src2, GPRReg dst)
+    {
+        setupArguments(src1, src2);
+        appendCall(operation);
+        move(GPRInfo::returnValueGPR, dst);
     }
 
     unsigned m_stackHeight;
@@ -240,6 +292,8 @@ private:
 
     Label m_beginLabel;
     Jump m_stackOverflow;
+    JumpList m_divideErrorJumpList;
+    JumpList m_exceptionChecks;
 
     Vector<std::pair<Call, void*>> m_calls;
 };
index be8c435..9a82f48 100644 (file)
@@ -430,6 +430,7 @@ ContextExpression WASMFunctionParser::parseExpressionI32(Context& context)
             return parseGetLocalExpressionI32(context);
         case WASMOpExpressionI32::Add:
         case WASMOpExpressionI32::Sub:
+        case WASMOpExpressionI32::SDiv:
             return parseBinaryExpressionI32(context, op);
         case WASMOpExpressionI32::ConstantPoolIndex:
         case WASMOpExpressionI32::GetGlobal:
@@ -460,7 +461,6 @@ ContextExpression WASMFunctionParser::parseExpressionI32(Context& context)
         case WASMOpExpressionI32::FromF64:
         case WASMOpExpressionI32::Negate:
         case WASMOpExpressionI32::Mul:
-        case WASMOpExpressionI32::SDiv:
         case WASMOpExpressionI32::UDiv:
         case WASMOpExpressionI32::SMod:
         case WASMOpExpressionI32::UMod: