Title: [205769] trunk/Source/_javascript_Core
Revision
205769
Author
keith_mil...@apple.com
Date
2016-09-09 14:30:51 -0700 (Fri, 09 Sep 2016)

Log Message

WASM should support if-then-else
https://bugs.webkit.org/show_bug.cgi?id=161778

Reviewed by Michael Saboff.

This patch makes some major changes to the way that the WASM
function parser works. First, the control stack has been moved
from the parser's context to the parser itself. This simplifies
the way that the parser works and allows us to make the decoder
iterative rather than recursive. Since the control stack has been
moved to the parser, any context operation that refers to some
block now receives that block by reference.

For any if block, regardless of whether or not it is an
if-then-else or not, we will allocate both the entire control flow
diamond. This is not a major issue in the if-then case since B3
will immediately cleanup these blocks. In order to support if-then
and if-then-else we needed to be able to distinguish what the type
of the top block on the control stack is. This will be necessary
when validating the else opcode in the future. In the B3 IR
generator we decide to the type of the block strictly by the
shape.

Currently, if blocks don't handle passed and returned stack values
correctly. I plan to fix this when I add support for the block
signatures. See: https://github.com/WebAssembly/design/pull/765

* testWASM.cpp:
(runWASMTests):
* wasm/WASMB3IRGenerator.cpp:
(dumpProcedure):
(JSC::WASM::parseAndCompile):
* wasm/WASMB3IRGenerator.h:
* wasm/WASMFunctionParser.h:
(JSC::WASM::FunctionParser<Context>::parseBlock):
(JSC::WASM::FunctionParser<Context>::parseExpression):
(JSC::WASM::FunctionParser<Context>::parseUnreachableExpression):
* wasm/WASMOps.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (205768 => 205769)


--- trunk/Source/_javascript_Core/ChangeLog	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-09-09 21:30:51 UTC (rev 205769)
@@ -1,3 +1,44 @@
+2016-09-08  Keith Miller  <keith_mil...@apple.com>
+
+        WASM should support if-then-else
+        https://bugs.webkit.org/show_bug.cgi?id=161778
+
+        Reviewed by Michael Saboff.
+
+        This patch makes some major changes to the way that the WASM
+        function parser works. First, the control stack has been moved
+        from the parser's context to the parser itself. This simplifies
+        the way that the parser works and allows us to make the decoder
+        iterative rather than recursive. Since the control stack has been
+        moved to the parser, any context operation that refers to some
+        block now receives that block by reference.
+
+        For any if block, regardless of whether or not it is an
+        if-then-else or not, we will allocate both the entire control flow
+        diamond. This is not a major issue in the if-then case since B3
+        will immediately cleanup these blocks. In order to support if-then
+        and if-then-else we needed to be able to distinguish what the type
+        of the top block on the control stack is. This will be necessary
+        when validating the else opcode in the future. In the B3 IR
+        generator we decide to the type of the block strictly by the
+        shape.
+
+        Currently, if blocks don't handle passed and returned stack values
+        correctly. I plan to fix this when I add support for the block
+        signatures. See: https://github.com/WebAssembly/design/pull/765
+
+        * testWASM.cpp:
+        (runWASMTests):
+        * wasm/WASMB3IRGenerator.cpp:
+        (dumpProcedure):
+        (JSC::WASM::parseAndCompile):
+        * wasm/WASMB3IRGenerator.h:
+        * wasm/WASMFunctionParser.h:
+        (JSC::WASM::FunctionParser<Context>::parseBlock):
+        (JSC::WASM::FunctionParser<Context>::parseExpression):
+        (JSC::WASM::FunctionParser<Context>::parseUnreachableExpression):
+        * wasm/WASMOps.h:
+
 2016-09-09  Filip Pizlo  <fpi...@apple.com>
 
         jsc.cpp should call initializeMainThread() to make sure that GC thread assertions work

Modified: trunk/Source/_javascript_Core/testWASM.cpp (205768 => 205769)


--- trunk/Source/_javascript_Core/testWASM.cpp	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/testWASM.cpp	2016-09-09 21:30:51 UTC (rev 205769)
@@ -241,6 +241,138 @@
 static void runWASMTests()
 {
     {
+        // Generated from:
+        //    (module
+        //     (func "dumb-eq" (param $x i32) (param $y i32) (result i32)
+        //      (if (i32.eq (get_local $x) (get_local $y))
+        //       (then (br 0))
+        //       (else (return (i32.const 1))))
+        //      (return (i32.const 0))
+        //      )
+        //     )
+        Vector<uint8_t> vector = {
+            0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x04, 0x74, 0x79, 0x70, 0x65, 0x87, 0x80, 0x80,
+            0x00, 0x01, 0x40, 0x02, 0x01, 0x01, 0x01, 0x01, 0x08, 0x66, 0x75, 0x6e, 0x63, 0x74, 0x69, 0x6f,
+            0x6e, 0x82, 0x80, 0x80, 0x00, 0x01, 0x00, 0x06, 0x65, 0x78, 0x70, 0x6f, 0x72, 0x74, 0x91, 0x80,
+            0x80, 0x00, 0x01, 0x00, 0x0e, 0x64, 0x75, 0x6d, 0x62, 0x2d, 0x6c, 0x65, 0x73, 0x73, 0x2d, 0x74,
+            0x68, 0x61, 0x6e, 0x04, 0x63, 0x6f, 0x64, 0x65, 0x97, 0x80, 0x80, 0x00, 0x01, 0x92, 0x80, 0x80,
+            0x00, 0x00, 0x14, 0x00, 0x14, 0x01, 0x4d, 0x03, 0x10, 0x00, 0x09, 0x01, 0x04, 0x10, 0x01, 0x09,
+            0x01, 0x0f, 0x0f
+        };
+
+        Plan plan(*vm, vector);
+        if (plan.result.size() != 1 || !plan.result[0]) {
+            dataLogLn("Module failed to compile correctly.");
+            CRASH();
+        }
+
+        // Test this doesn't crash.
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(0), box(1) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(0) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(1) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(2) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(2) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(1) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(6) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(100), box(6) }), 1);
+    }
+
+    {
+        // Generated from:
+        //    (module
+        //     (func "dumb-less-than" (param $x i32) (param $y i32) (result i32)
+        //      (loop
+        //       (if (i32.eq (get_local $x) (get_local $y))
+        //        (then (return (i32.const 0)))
+        //        (else (if (i32.eq (get_local $x) (i32.const 0))
+        //               (return (i32.const 1)))))
+        //       (set_local $x (i32.sub (get_local $x) (i32.const 1)))
+        //       (br 0)
+        //       )
+        //      )
+        //     )
+        Vector<uint8_t> vector = {
+            0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x04, 0x74, 0x79, 0x70, 0x65, 0x87, 0x80, 0x80,
+            0x00, 0x01, 0x40, 0x02, 0x01, 0x01, 0x01, 0x01, 0x08, 0x66, 0x75, 0x6e, 0x63, 0x74, 0x69, 0x6f,
+            0x6e, 0x82, 0x80, 0x80, 0x00, 0x01, 0x00, 0x06, 0x65, 0x78, 0x70, 0x6f, 0x72, 0x74, 0x91, 0x80,
+            0x80, 0x00, 0x01, 0x00, 0x0e, 0x64, 0x75, 0x6d, 0x62, 0x2d, 0x6c, 0x65, 0x73, 0x73, 0x2d, 0x74,
+            0x68, 0x61, 0x6e, 0x04, 0x63, 0x6f, 0x64, 0x65, 0xaa, 0x80, 0x80, 0x00, 0x01, 0xa5, 0x80, 0x80,
+            0x00, 0x00, 0x02, 0x14, 0x00, 0x14, 0x01, 0x4d, 0x03, 0x10, 0x00, 0x09, 0x01, 0x04, 0x14, 0x00,
+            0x10, 0x00, 0x4d, 0x03, 0x10, 0x01, 0x09, 0x01, 0x0f, 0x0f, 0x14, 0x00, 0x10, 0x01, 0x41, 0x15,
+            0x00, 0x06, 0x00, 0x00, 0x0f, 0x0f
+        };
+
+        Plan plan(*vm, vector);
+        if (plan.result.size() != 1 || !plan.result[0]) {
+            dataLogLn("Module failed to compile correctly.");
+            CRASH();
+        }
+
+        // Test this doesn't crash.
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(0), box(1) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(0) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(1) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(2) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(2) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(1) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(6) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(100), box(6) }), 0);
+    }
+
+    {
+        // Generated from:
+        //    (module
+        //     (func "dumb-less-than" (param $x i32) (param $y i32) (result i32)
+        //      (loop
+        //       (block
+        //        (block
+        //         (br_if 0 (i32.eq (get_local $x) (i32.const 0)))
+        //         (br 1)
+        //         )
+        //        (return (i32.const 1))
+        //        )
+        //       (block
+        //        (block
+        //         (br_if 0 (i32.eq (get_local $x) (get_local $y)))
+        //         (br 1)
+        //         )
+        //        (return (i32.const 0))
+        //        )
+        //       (set_local $x (i32.sub (get_local $x) (i32.const 1)))
+        //       (br 0)
+        //       )
+        //      )
+        //     )
+        Vector<uint8_t> vector = {
+            0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x04, 0x74, 0x79, 0x70, 0x65, 0x87, 0x80, 0x80,
+            0x00, 0x01, 0x40, 0x02, 0x01, 0x01, 0x01, 0x01, 0x08, 0x66, 0x75, 0x6e, 0x63, 0x74, 0x69, 0x6f,
+            0x6e, 0x82, 0x80, 0x80, 0x00, 0x01, 0x00, 0x06, 0x65, 0x78, 0x70, 0x6f, 0x72, 0x74, 0x91, 0x80,
+            0x80, 0x00, 0x01, 0x00, 0x0e, 0x64, 0x75, 0x6d, 0x62, 0x2d, 0x6c, 0x65, 0x73, 0x73, 0x2d, 0x74,
+            0x68, 0x61, 0x6e, 0x04, 0x63, 0x6f, 0x64, 0x65, 0xb9, 0x80, 0x80, 0x00, 0x01, 0xb4, 0x80, 0x80,
+            0x00, 0x00, 0x02, 0x01, 0x01, 0x14, 0x00, 0x14, 0x01, 0x4d, 0x07, 0x00, 0x00, 0x06, 0x00, 0x01,
+            0x0f, 0x10, 0x00, 0x09, 0x01, 0x0f, 0x01, 0x01, 0x14, 0x00, 0x10, 0x00, 0x4d, 0x07, 0x00, 0x00,
+            0x06, 0x00, 0x01, 0x0f, 0x10, 0x01, 0x09, 0x01, 0x0f, 0x14, 0x00, 0x10, 0x01, 0x41, 0x15, 0x00,
+            0x06, 0x00, 0x00, 0x0f, 0x0f
+        };
+
+        Plan plan(*vm, vector);
+        if (plan.result.size() != 1 || !plan.result[0]) {
+            dataLogLn("Module failed to compile correctly.");
+            CRASH();
+        }
+
+        // Test this doesn't crash.
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(0), box(1) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(0) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(1) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(2) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(2) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(1), box(1) }), 0);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(2), box(6) }), 1);
+        CHECK_EQ(invoke<int>(*plan.result[0], { box(100), box(6) }), 0);
+    }
+
+    {
         // Generated from: (module (func "return-i32" (result i32) (return (i32.const 5))) )
         Vector<uint8_t> vector = {
             0x00, 0x61, 0x73, 0x6d, 0x0c, 0x00, 0x00, 0x00, 0x04, 0x74, 0x79, 0x70, 0x65, 0x85, 0x80, 0x80,

Modified: trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.cpp (205768 => 205769)


--- trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.cpp	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.cpp	2016-09-09 21:30:51 UTC (rev 205769)
@@ -39,6 +39,12 @@
 #include "WASMFunctionParser.h"
 #include <wtf/Optional.h>
 
+void dumpProcedure(void* ptr)
+{
+    JSC::B3::Procedure* proc = static_cast<JSC::B3::Procedure*>(ptr);
+    proc->dump(WTF::dataFile());
+}
+
 namespace JSC { namespace WASM {
 
 namespace {
@@ -71,8 +77,13 @@
 private:
     class LazyBlock {
     public:
-        explicit operator bool() { return !!m_block; }
+        LazyBlock(BasicBlock* block)
+            : m_block(block)
+        {
+        }
 
+        explicit operator bool() const { return !!m_block; }
+
         BasicBlock* get(Procedure& proc)
         {
             if (!m_block)
@@ -92,9 +103,11 @@
         BasicBlock* m_block { nullptr };
     };
 
+public:
     struct ControlData {
-        ControlData(Optional<Vector<Variable*>>&& stack, BasicBlock* loopTarget = nullptr)
-            : loopTarget(loopTarget)
+        ControlData(Optional<Vector<Variable*>>&& stack, BasicBlock* special = nullptr, BasicBlock* continuation = nullptr)
+            : continuation(continuation)
+            , special(special)
             , stack(stack)
         {
         }
@@ -101,32 +114,52 @@
 
         void dump(PrintStream& out) const
         {
-            out.print("Continuation: ", continuation, ", Target: ");
-            if (loopTarget)
-                out.print(*loopTarget);
+            switch (type()) {
+            case BlockType::If:
+                out.print("If:    ");
+                break;
+            case BlockType::Block:
+                out.print("Block: ");
+                break;
+            case BlockType::Loop:
+                out.print("Loop:  ");
+                break;
+            }
+            out.print("Continuation: ", continuation, ", Special: ");
+            if (special)
+                out.print(*special);
             else
-                out.print(continuation);
+                out.print("None");
         }
 
+        BlockType type() const
+        {
+            if (!special)
+                return BlockType::Block;
+            if (continuation)
+                return BlockType::If;
+            return BlockType::Loop;
+        }
+
         BasicBlock* targetBlockForBranch(Procedure& proc)
         {
-            if (loopTarget)
-                return loopTarget;
+            if (special)
+                return special;
             return continuation.get(proc);
         }
 
-        bool isLoop() { return !!loopTarget; }
-
+    private:
+        friend class B3IRGenerator;
         // We use a LazyBlock for the continuation since B3::validate does not like orphaned blocks. Note,
         // it's possible to create an orphaned block by doing something like (block (return (...))). In
         // that example, if we eagerly allocate a BasicBlock for the continuation it will never be reachable.
         LazyBlock continuation;
-        BasicBlock* loopTarget;
+        BasicBlock* special;
         Optional<Vector<Variable*>> stack;
     };
 
-public:
     typedef Value* ExpressionType;
+    typedef ControlData ControlType;
     static constexpr ExpressionType emptyExpression = nullptr;
 
     B3IRGenerator(Procedure&);
@@ -141,29 +174,26 @@
     bool WARN_UNUSED_RETURN binaryOp(BinaryOpType, ExpressionType left, ExpressionType right, ExpressionType& result);
     bool WARN_UNUSED_RETURN unaryOp(UnaryOpType, ExpressionType arg, ExpressionType& result);
 
-    bool WARN_UNUSED_RETURN addBlock();
-    bool WARN_UNUSED_RETURN addLoop();
-    bool WARN_UNUSED_RETURN endBlock(Vector<ExpressionType, 1>& expressionStack);
+    ControlData WARN_UNUSED_RETURN addBlock();
+    ControlData WARN_UNUSED_RETURN addLoop();
+    ControlData WARN_UNUSED_RETURN addIf(ExpressionType condition);
+    bool WARN_UNUSED_RETURN addElse(ControlData&);
 
     bool WARN_UNUSED_RETURN addReturn(const Vector<ExpressionType, 1>& returnValues);
-    bool WARN_UNUSED_RETURN addBranch(ExpressionType condition, const Vector<ExpressionType, 1>& returnValues, uint32_t target);
+    bool WARN_UNUSED_RETURN addBranch(ControlData&, ExpressionType condition, const Vector<ExpressionType, 1>& returnValues);
+    bool WARN_UNUSED_RETURN endBlock(ControlData&, Vector<ExpressionType, 1>& expressionStack);
 
-    void dumpGraphAndControlStack();
+    bool isContinuationReachable(ControlData&);
 
+    void dump(const Vector<ControlType>& controlStack, const Vector<ExpressionType, 1>& expressionStack);
+
 private:
-    ControlData& controlDataForLevel(unsigned);
     void unify(Variable* target, const ExpressionType source);
     Vector<Variable*> initializeIncommingTypes(BasicBlock*, const Vector<ExpressionType, 1>&);
     void unifyValuesWithBlock(const Vector<ExpressionType, 1>& resultStack, Optional<Vector<Variable*>>& stack, BasicBlock* target);
 
-public:
-    unsigned unreachable { 0 };
-
-private:
     Procedure& m_proc;
     BasicBlock* m_currentBlock;
-    // This is a pair of the continuation and the types expected on the stack for that continuation.
-    Vector<ControlData> m_controlStack;
     Vector<Variable*> m_locals;
 };
 
@@ -235,109 +265,59 @@
     }
 }
 
-bool B3IRGenerator::addBlock()
+B3IRGenerator::ControlData B3IRGenerator::addBlock()
 {
-    if (unreachable) {
-        unreachable++;
-        return true;
-    }
-
-    if (verbose) {
-        dataLogLn("Adding block");
-        dumpGraphAndControlStack();
-    }
-    m_controlStack.append(ControlData(Nullopt));
-
-    return true;
+    return ControlData(Nullopt);
 }
 
-bool B3IRGenerator::addLoop()
+B3IRGenerator::ControlData B3IRGenerator::addLoop()
 {
-    if (unreachable) {
-        unreachable++;
-        return true;
-    }
-
-    if (verbose) {
-        dataLogLn("Adding loop");
-        dumpGraphAndControlStack();
-    }
     BasicBlock* body = m_proc.addBlock();
     m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), body);
     body->addPredecessor(m_currentBlock);
     m_currentBlock = body;
-    m_controlStack.append(ControlData(Vector<Variable*>(), body));
-    return true;
+    return ControlData(Vector<Variable*>(), body);
 }
 
-bool B3IRGenerator::endBlock(Vector<ExpressionType, 1>& expressionStack)
+B3IRGenerator::ControlData B3IRGenerator::addIf(ExpressionType condition)
 {
-    if (unreachable > 1) {
-        unreachable--;
-        return true;
-    }
+    // FIXME: This needs to do some kind of stack passing.
 
-    if (verbose) {
-        dataLogLn("Falling out of block");
-        dumpGraphAndControlStack();
-    }
-    // This means that we are exiting the function.
-    if (!m_controlStack.size()) {
-        // FIXME: Should this require the stack is empty? It's not clear from the current spec.
-        return !expressionStack.size();
-    }
+    BasicBlock* taken = m_proc.addBlock();
+    BasicBlock* notTaken = m_proc.addBlock();
+    BasicBlock* continuation = m_proc.addBlock();
 
-    ControlData data = ""
-    if (unreachable) {
-        // If nothing targets the continuation of the current block then we don't want to create
-        // an orphaned BasicBlock since it can't be reached by fallthrough.
-        if (data.continuation) {
-            m_currentBlock = data.continuation.get(m_proc);
-            unreachable--;
-        }
-        return true;
-    }
+    m_currentBlock->appendNew<Value>(m_proc, B3::Branch, Origin(), condition);
+    m_currentBlock->setSuccessors(FrequentedBlock(taken), FrequentedBlock(notTaken));
+    taken->addPredecessor(m_currentBlock);
+    notTaken->addPredecessor(m_currentBlock);
 
-    BasicBlock* continuation = data.continuation.get(m_proc);
-    unifyValuesWithBlock(expressionStack, data.stack, continuation);
-    m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), continuation);
-    continuation->addPredecessor(m_currentBlock);
-    m_currentBlock = continuation;
+    m_currentBlock = taken;
+    return ControlData(Nullopt, notTaken, continuation);
+}
+
+bool B3IRGenerator::addElse(ControlData& data)
+{
+    ASSERT(data.continuation);
+    m_currentBlock = data.special;
+    // Clear the special pointer so that when we parse the end we don't think that this block is an if block.
+    data.special = nullptr;
+    ASSERT(data.type() == BlockType::Block);
     return true;
 }
 
 bool B3IRGenerator::addReturn(const Vector<ExpressionType, 1>& returnValues)
 {
-    if (unreachable)
-        return true;
-
-    if (verbose) {
-        dataLogLn("Adding return");
-        dumpGraphAndControlStack();
-    }
-
     ASSERT(returnValues.size() <= 1);
     if (returnValues.size())
         m_currentBlock->appendNewControlValue(m_proc, B3::Return, Origin(), returnValues[0]);
     else
         m_currentBlock->appendNewControlValue(m_proc, B3::Return, Origin());
-    unreachable = 1;
     return true;
 }
 
-bool B3IRGenerator::addBranch(ExpressionType condition, const Vector<ExpressionType, 1>& returnValues, uint32_t level)
+bool B3IRGenerator::addBranch(ControlData& data, ExpressionType condition, const Vector<ExpressionType, 1>& returnValues)
 {
-    if (unreachable)
-        return true;
-
-    ASSERT(level < m_controlStack.size());
-    ControlData& data = ""
-    if (verbose) {
-        dataLogLn("Adding Branch from: ",  *m_currentBlock, " targeting: ", level, " with data: ", data);
-        dumpGraphAndControlStack();
-    }
-
-
     BasicBlock* target = data.targetBlockForBranch(m_proc);
     unifyValuesWithBlock(returnValues, data.stack, target);
     if (condition) {
@@ -350,12 +330,48 @@
     } else {
         m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), FrequentedBlock(target));
         target->addPredecessor(m_currentBlock);
-        unreachable = 1;
     }
 
     return true;
 }
 
+bool B3IRGenerator::endBlock(ControlData& data, Vector<ExpressionType, 1>& expressionStack)
+{
+    if (!data.continuation)
+        return true;
+
+    BasicBlock* continuation = data.continuation.get(m_proc);
+    if (data.type() == BlockType::If) {
+        ASSERT(!data.special->size() && !data.special->successors().size());
+        // Since we don't have any else block we need to point the notTaken branch to the continuation.
+        data.special->appendNewControlValue(m_proc, Jump, Origin());
+        data.special->setSuccessors(FrequentedBlock(continuation));
+        continuation->addPredecessor(data.special);
+    }
+
+    unifyValuesWithBlock(expressionStack, data.stack, continuation);
+    m_currentBlock->appendNewControlValue(m_proc, Jump, Origin(), continuation);
+    continuation->addPredecessor(m_currentBlock);
+    m_currentBlock = continuation;
+    return true;
+}
+
+bool B3IRGenerator::isContinuationReachable(ControlData& data)
+{
+    // If nothing targets the continuation of the current block then we don't want to create
+    // an orphaned BasicBlock since it can't be reached by fallthrough.
+    if (!data.continuation)
+        return false;
+
+    m_currentBlock = data.continuation.get(m_proc);
+    if (data.type() == BlockType::If) {
+        data.special->appendNewControlValue(m_proc, Jump, Origin(), m_currentBlock);
+        m_currentBlock->addPredecessor(data.special);
+    }
+
+    return true;
+}
+
 void B3IRGenerator::unify(Variable* variable, ExpressionType source)
 {
     m_currentBlock->appendNew<VariableValue>(m_proc, Set, Origin(), variable, source);
@@ -387,20 +403,18 @@
     for (size_t i = 0; i < resultStack.size(); ++i)
         unify(stack.value()[i], resultStack[i]);
 }
-    
-B3IRGenerator::ControlData& B3IRGenerator::controlDataForLevel(unsigned level)
-{
-    return m_controlStack[m_controlStack.size() - 1 - level];
-}
 
-void B3IRGenerator::dumpGraphAndControlStack()
+void B3IRGenerator::dump(const Vector<ControlType>& controlStack, const Vector<ExpressionType, 1>& expressionStack)
 {
     dataLogLn("Processing Graph:");
     dataLog(m_proc);
     dataLogLn("With current block:", *m_currentBlock);
-    dataLogLn("With Control Stack:");
-    for (unsigned i = 0; i < m_controlStack.size(); ++i)
-        dataLogLn("[", i, "] ", m_controlStack[i]);
+    dataLogLn("Control stack:");
+    for (const ControlType& data : controlStack)
+        dataLogLn("  ", data);
+    dataLogLn("ExpressionStack:");
+    for (const ExpressionType& _expression_ : expressionStack)
+        dataLogLn("  ", *_expression_);
     dataLogLn("\n");
 }
 
@@ -414,6 +428,7 @@
     if (!parser.parse())
         RELEASE_ASSERT_NOT_REACHED();
 
+    procedure.resetReachability();
     validate(procedure, "After parsing:\n");
 
     fixSSA(procedure);

Modified: trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.h (205768 => 205769)


--- trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.h	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/wasm/WASMB3IRGenerator.h	2016-09-09 21:30:51 UTC (rev 205769)
@@ -31,6 +31,8 @@
 #include "VM.h"
 #include "WASMFormat.h"
 
+extern "C" void dumpProcedure(void*);
+
 namespace JSC { namespace WASM {
 
 std::unique_ptr<B3::Compilation> parseAndCompile(VM&, Vector<uint8_t>&, FunctionInformation, unsigned optLevel = 1);

Modified: trunk/Source/_javascript_Core/wasm/WASMFunctionParser.h (205768 => 205769)


--- trunk/Source/_javascript_Core/wasm/WASMFunctionParser.h	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/wasm/WASMFunctionParser.h	2016-09-09 21:30:51 UTC (rev 205769)
@@ -32,10 +32,17 @@
 
 namespace JSC { namespace WASM {
 
+enum class BlockType {
+    If,
+    Block,
+    Loop
+};
+
 template<typename Context>
 class FunctionParser : public Parser {
 public:
     typedef typename Context::ExpressionType ExpressionType;
+    typedef typename Context::ControlType ControlType;
 
     FunctionParser(Context&, const Vector<uint8_t>& sourceBuffer, const FunctionInformation&);
 
@@ -46,12 +53,13 @@
 
     bool WARN_UNUSED_RETURN parseBlock();
     bool WARN_UNUSED_RETURN parseExpression(OpType);
+    bool WARN_UNUSED_RETURN parseUnreachableExpression(OpType);
     bool WARN_UNUSED_RETURN unifyControl(Vector<ExpressionType>&, unsigned level);
 
-    Optional<Vector<ExpressionType>>& stackForControlLevel(unsigned level);
-
     Context& m_context;
     Vector<ExpressionType, 1> m_expressionStack;
+    Vector<ControlType> m_controlStack;
+    unsigned m_unreachableBlocks { 0 };
 };
 
 template<typename Context>
@@ -92,27 +100,37 @@
         if (!parseUInt7(op))
             return false;
 
-        if (!parseExpression(static_cast<OpType>(op))) {
+        if (verbose) {
+            dataLogLn("processing op (", m_unreachableBlocks, "): ",  RawPointer(reinterpret_cast<void*>(op)));
+            m_context.dump(m_controlStack, m_expressionStack);
+        }
+
+        if (op == OpType::End && !m_controlStack.size())
+            break;
+
+        if (m_unreachableBlocks) {
+            if (!parseUnreachableExpression(static_cast<OpType>(op))) {
+                if (verbose)
+                    dataLogLn("failed to process unreachable op:", op);
+                return false;
+            }
+        } else if (!parseExpression(static_cast<OpType>(op))) {
             if (verbose)
                 dataLogLn("failed to process op:", op);
             return false;
         }
 
-        if (op == OpType::End)
-            break;
     }
 
+    // I'm not sure if we should check the _expression_ stack here...
     return true;
 }
+#define CREATE_CASE(name, id, b3op) case name:
 
 template<typename Context>
 bool FunctionParser<Context>::parseExpression(OpType op)
 {
-    if (m_context.unreachable && !isControlOp(op))
-        return true;
-
     switch (op) {
-#define CREATE_CASE(name, id, b3op) case name:
     FOR_EACH_WASM_BINARY_OP(CREATE_CASE) {
         ExpressionType right = m_expressionStack.takeLast();
         ExpressionType left = m_expressionStack.takeLast();
@@ -131,7 +149,6 @@
         m_expressionStack.append(result);
         return true;
     }
-#undef CREATE_CASE
 
     case OpType::I32Const: {
         uint32_t constant;
@@ -162,17 +179,25 @@
     }
 
     case OpType::Block: {
-        if (!m_context.addBlock())
-            return false;
-        return parseBlock();
+        m_controlStack.append(m_context.addBlock());
+        return true;
     }
 
     case OpType::Loop: {
-        if (!m_context.addLoop())
-            return false;
-        return parseBlock();
+        m_controlStack.append(m_context.addLoop());
+        return true;
     }
 
+    case OpType::If: {
+        ExpressionType condition = m_expressionStack.takeLast();
+        m_controlStack.append(m_context.addIf(condition));
+        return true;
+    }
+
+    case OpType::Else: {
+        return m_context.addElse(m_controlStack.last());
+    }
+
     case OpType::Branch:
     case OpType::BranchIf: {
         uint32_t arity;
@@ -186,13 +211,18 @@
         ExpressionType condition = Context::emptyExpression;
         if (op == OpType::BranchIf)
             condition = m_expressionStack.takeLast();
+        else
+            m_unreachableBlocks = 1;
 
-
         Vector<ExpressionType, 1> values(arity);
         for (unsigned i = arity; i; i--)
             values[i-1] = m_expressionStack.takeLast();
 
-        return m_context.addBranch(condition, values, target);
+        if (target >= m_controlStack.size())
+            return false;
+        ControlType& data = "" - 1 - target];
+
+        return m_context.addBranch(data, condition, values);
     }
 
     case OpType::Return: {
@@ -203,11 +233,14 @@
         if (returnCount)
             returnValues.append(m_expressionStack.takeLast());
 
+        m_unreachableBlocks = 1;
         return m_context.addReturn(returnValues);
     }
 
-    case OpType::End:
-        return m_context.endBlock(m_expressionStack);
+    case OpType::End: {
+        ControlType data = ""
+        return m_context.endBlock(data, m_expressionStack);
+    }
 
     }
 
@@ -215,6 +248,64 @@
     return false;
 }
 
+template<typename Context>
+bool FunctionParser<Context>::parseUnreachableExpression(OpType op)
+{
+    ASSERT(m_unreachableBlocks);
+    switch (op) {
+    case OpType::Else: {
+        if (m_unreachableBlocks > 1)
+            return true;
+
+        ControlType& data = ""
+        ASSERT(data.type() == BlockType::If);
+        m_unreachableBlocks = 0;
+        return m_context.addElse(data);
+    }
+
+    case OpType::End: {
+        if (m_unreachableBlocks == 1) {
+            ControlType data = ""
+            if (!m_context.isContinuationReachable(data))
+                return true;
+        }
+        m_unreachableBlocks--;
+        return true;
+    }
+
+    case OpType::Loop:
+    case OpType::If:
+    case OpType::Block: {
+        m_unreachableBlocks++;
+        return true;
+    }
+
+    // two immediate cases
+    case OpType::Branch:
+    case OpType::BranchIf: {
+        uint32_t unused;
+        if (!parseVarUInt32(unused))
+            return false;
+        return parseVarUInt32(unused);
+    }
+
+    // one immediate cases
+    case OpType::Return:
+    case OpType::I32Const:
+    case OpType::SetLocal:
+    case OpType::GetLocal: {
+        uint32_t unused;
+        return parseVarUInt32(unused);
+    }
+
+    default:
+        break;
+    }
+    return true;
+}
+
+#undef CREATE_CASE
+
 } } // namespace JSC::WASM
 
 #endif // ENABLE(WEBASSEMBLY)

Modified: trunk/Source/_javascript_Core/wasm/WASMOps.h (205768 => 205769)


--- trunk/Source/_javascript_Core/wasm/WASMOps.h	2016-09-09 21:29:47 UTC (rev 205768)
+++ trunk/Source/_javascript_Core/wasm/WASMOps.h	2016-09-09 21:30:51 UTC (rev 205769)
@@ -37,6 +37,8 @@
 #define FOR_EACH_WASM_CONTROL_FLOW_OP(macro) \
     macro(Block, 0x01, NA) \
     macro(Loop, 0x02, NA) \
+    macro(If, 0x03, NA) \
+    macro(Else, 0x04, NA) \
     macro(Branch, 0x06, NA) \
     macro(BranchIf, 0x07, NA) \
     macro(Return, 0x09, NA) \
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to