Bug 1328639: wasm: Simplify block-value popping in Ion compilation; r=luke
authorBenjamin Bouvier <benj@benj.me>
Wed, 04 Jan 2017 18:17:29 +0100
changeset 356143 d2e85dc48786918c91c9bfd1d44fed9fb9ddc5c4
parent 356142 0fe0b5c910f78edfd08b4230393f75968924dd0b
child 356144 3b469ed8f8192eea438afe2144298a5e71603566
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1328639
milestone53.0a1
Bug 1328639: wasm: Simplify block-value popping in Ion compilation; r=luke MozReview-Commit-ID: LRN3CWT91Lt
js/src/wasm/WasmIonCompile.cpp
--- a/js/src/wasm/WasmIonCompile.cpp
+++ b/js/src/wasm/WasmIonCompile.cpp
@@ -153,18 +153,16 @@ class FunctionCompiler
     const ValTypeVector&       locals_;
     size_t                     lastReadCallSite_;
 
     TempAllocator&             alloc_;
     MIRGraph&                  graph_;
     const CompileInfo&         info_;
     MIRGenerator&              mirGen_;
 
-    MInstruction*              dummyIns_;
-
     MBasicBlock*               curBlock_;
     CallCompileStateVector     callStack_;
     uint32_t                   maxStackArgBytes_;
 
     uint32_t                   loopDepth_;
     uint32_t                   blockDepth_;
     ControlFlowPatchsVector    blockPatches_;
 
@@ -181,17 +179,16 @@ class FunctionCompiler
         iter_(decoder, func.lineOrBytecode()),
         func_(func),
         locals_(locals),
         lastReadCallSite_(0),
         alloc_(mirGen.alloc()),
         graph_(mirGen.graph()),
         info_(mirGen.info()),
         mirGen_(mirGen),
-        dummyIns_(nullptr),
         curBlock_(nullptr),
         maxStackArgBytes_(0),
         loopDepth_(0),
         blockDepth_(0),
         tlsPointer_(nullptr)
     {}
 
     const ModuleEnvironment&   env() const   { return env_; }
@@ -273,19 +270,16 @@ class FunctionCompiler
             }
 
             curBlock_->add(ins);
             curBlock_->initSlot(info().localSlot(i), ins);
             if (!mirGen_.ensureBallast())
                 return false;
         }
 
-        dummyIns_ = MConstant::New(alloc(), Int32Value(0), MIRType::Int32);
-        curBlock_->add(dummyIns_);
-
         addInterruptCheck();
 
         return true;
     }
 
     void finish()
     {
         mirGen().initWasmMaxStackArgBytes(maxStackArgBytes_);
@@ -1157,55 +1151,23 @@ class FunctionCompiler
     {
         if (inDeadCode())
             return;
         MOZ_ASSERT(!hasPushed(curBlock_));
         if (def && def->type() != MIRType::None)
             curBlock_->push(def);
     }
 
-    MDefinition* popDefIfPushed(bool shouldReturn = true)
+    MDefinition* popDefIfPushed()
     {
         if (!hasPushed(curBlock_))
             return nullptr;
         MDefinition* def = curBlock_->pop();
-        MOZ_ASSERT_IF(def->type() == MIRType::Value, !shouldReturn);
-        return shouldReturn ? def : nullptr;
-    }
-
-    template <typename GetBlock>
-    bool ensurePushInvariants(const GetBlock& getBlock, size_t numBlocks)
-    {
-        // Preserve the invariant that, for every iterated MBasicBlock, either:
-        // every MBasicBlock has a pushed expression with the same type (to
-        // prevent creating used phis with type Value) OR no MBasicBlock has any
-        // pushed expression. This is required by MBasicBlock::addPredecessor.
-        if (numBlocks < 2)
-            return true;
-
-        MBasicBlock* block = getBlock(0);
-
-        bool allPushed = hasPushed(block);
-        if (allPushed) {
-            MIRType type = peekPushedDef(block)->type();
-            for (size_t i = 1; allPushed && i < numBlocks; i++) {
-                block = getBlock(i);
-                allPushed = hasPushed(block) && peekPushedDef(block)->type() == type;
-            }
-        }
-
-        if (!allPushed) {
-            for (size_t i = 0; i < numBlocks; i++) {
-                block = getBlock(i);
-                if (!hasPushed(block))
-                    block->push(dummyIns_);
-            }
-        }
-
-        return allPushed;
+        MOZ_ASSERT(def->type() != MIRType::Value);
+        return def;
     }
 
   private:
     void addJoinPredecessor(MDefinition* def, MBasicBlock** joinPred)
     {
         *joinPred = curBlock_;
         if (inDeadCode())
             return;
@@ -1265,34 +1227,31 @@ class FunctionCompiler
 
             mozilla::Array<MBasicBlock*, 2> blocks;
             size_t numJoinPreds = 0;
             if (thenJoinPred)
                 blocks[numJoinPreds++] = thenJoinPred;
             if (elseJoinPred)
                 blocks[numJoinPreds++] = elseJoinPred;
 
-            auto getBlock = [&](size_t i) -> MBasicBlock* { return blocks[i]; };
-            bool yieldsValue = ensurePushInvariants(getBlock, numJoinPreds);
-
             if (numJoinPreds == 0) {
                 *def = nullptr;
                 return true;
             }
 
             MBasicBlock* join;
             if (!goToNewBlock(blocks[0], &join))
                 return false;
             for (size_t i = 1; i < numJoinPreds; ++i) {
                 if (!goToExistingBlock(blocks[i], join))
                     return false;
             }
 
             curBlock_ = join;
-            *def = popDefIfPushed(yieldsValue);
+            *def = popDefIfPushed();
         }
 
         return true;
     }
 
     bool startBlock()
     {
         MOZ_ASSERT_IF(blockDepth_ < blockPatches_.length(), blockPatches_[blockDepth_].empty());
@@ -1586,28 +1545,20 @@ class FunctionCompiler
     bool bindBranches(uint32_t absolute, MDefinition** def)
     {
         if (absolute >= blockPatches_.length() || blockPatches_[absolute].empty()) {
             *def = inDeadCode() ? nullptr : popDefIfPushed();
             return true;
         }
 
         ControlFlowPatchVector& patches = blockPatches_[absolute];
-
-        auto getBlock = [&](size_t i) -> MBasicBlock* {
-            if (i < patches.length())
-                return patches[i].ins->block();
-            return curBlock_;
-        };
-
-        bool yieldsValue = ensurePushInvariants(getBlock, patches.length() + !!curBlock_);
+        MControlInstruction* ins = patches[0].ins;
+        MBasicBlock* pred = ins->block();
 
         MBasicBlock* join = nullptr;
-        MControlInstruction* ins = patches[0].ins;
-        MBasicBlock* pred = ins->block();
         if (!newBlock(pred, &join))
             return false;
 
         pred->mark();
         ins->replaceSuccessor(patches[0].index, join);
 
         for (size_t i = 1; i < patches.length(); i++) {
             ins = patches[i].ins;
@@ -1626,17 +1577,17 @@ class FunctionCompiler
         for (uint32_t i = 0; i < join->numPredecessors(); i++)
             join->getPredecessor(i)->unmark();
 
         if (curBlock_ && !goToExistingBlock(curBlock_, join))
             return false;
 
         curBlock_ = join;
 
-        *def = popDefIfPushed(yieldsValue);
+        *def = popDefIfPushed();
 
         patches.clear();
         return true;
     }
 };
 
 template <>
 MDefinition* FunctionCompiler::unary<MToFloat32>(MDefinition* op)