Bug 1263203: Ensure we don't have Phi values in wasm; r=luke
authorBenjamin Bouvier <benj@benj.me>
Fri, 08 Apr 2016 19:29:22 +0200
changeset 317358 6828e2e05e1690cc36b1e282ee2f997ffce12e1f
parent 317357 1d139b7422c8bad036d3b21308e0ceef87ae1cf8
child 317359 bc5264d6536ed11cc513fd003453e0a91a3f45ca
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1263203
milestone48.0a1
Bug 1263203: Ensure we don't have Phi values in wasm; r=luke MozReview-Commit-ID: 5XKeBIJbpN0
js/src/asmjs/WasmIonCompile.cpp
js/src/jit-test/tests/wasm/random-control-flow.js
--- a/js/src/asmjs/WasmIonCompile.cpp
+++ b/js/src/asmjs/WasmIonCompile.cpp
@@ -953,41 +953,65 @@ class FunctionCompiler
   private:
     static bool hasPushed(MBasicBlock* block)
     {
         uint32_t numPushed = block->stackDepth() - block->info().firstStackSlot();
         MOZ_ASSERT(numPushed == 0 || numPushed == 1);
         return numPushed;
     }
 
+    static MDefinition* peekPushedDef(MBasicBlock* block)
+    {
+        MOZ_ASSERT(hasPushed(block));
+        return block->getSlot(block->stackDepth() - 1);
+    }
+
   public:
     void pushDef(MDefinition* def)
     {
         if (inDeadCode())
             return;
         MOZ_ASSERT(!hasPushed(curBlock_));
         if (def && def->type() != MIRType_None)
             curBlock_->push(def);
     }
 
+    MDefinition* popDefIfPushed()
+    {
+        if (!hasPushed(curBlock_))
+            return nullptr;
+        MDefinition* def = curBlock_->pop();
+        MOZ_ASSERT(def->type() != MIRType_Value);
+        return def;
+    }
+
     template <typename GetBlock>
     void ensurePushInvariants(const GetBlock& getBlock, size_t numBlocks)
     {
-        // Preserve the invariant that, for every iterated MBasicBlock,
-        // either: every MBasicBlock has a non-void pushed expression OR no
-        // MBasicBlock has any pushed expression. This is required by
-        // MBasicBlock::addPredecessor.
-        bool allPushed = true;
-
-        for (size_t i = 0; allPushed && i < numBlocks; i++)
-            allPushed = hasPushed(getBlock(i));
+        // Preserve the invariant that, for every iterated MBasicBlock, either:
+        // every MBasicBlock has a pushed expression with the same type (to
+        // prevent creating phis with type Value) OR no MBasicBlock has any
+        // pushed expression. This is required by MBasicBlock::addPredecessor.
+        if (numBlocks < 2)
+            return;
+
+        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++) {
-                MBasicBlock* block = getBlock(i);
+                block = getBlock(i);
                 if (hasPushed(block))
                     block->pop();
             }
         }
     }
 
     bool joinIf(MBasicBlock* joinBlock, BlockVector* blocks, MDefinition** def)
     {
@@ -1029,20 +1053,17 @@ class FunctionCompiler
         if (!goToNewBlock((*blocks)[0], &join))
             return false;
         for (size_t i = 1; i < blocks->length(); i++) {
             if (!goToExistingBlock((*blocks)[i], join))
                 return false;
         }
 
         curBlock_ = join;
-        if (hasPushed(curBlock_))
-            *def = curBlock_->pop();
-        else
-            *def = nullptr;
+        *def = popDefIfPushed();
         return true;
     }
 
     bool startBlock()
     {
         MOZ_ASSERT_IF(blockDepth_ < blockPatches_.length(), blockPatches_[blockDepth_].empty());
         blockDepth_++;
         return true;
@@ -1352,17 +1373,17 @@ class FunctionCompiler
         MOZ_ASSERT(next);
         prev->end(MGoto::New(alloc(), next));
         return next->addPredecessor(alloc(), prev);
     }
 
     bool bindBranches(uint32_t absolute, MDefinition** def)
     {
         if (absolute >= blockPatches_.length() || blockPatches_[absolute].empty()) {
-            *def = !inDeadCode() && hasPushed(curBlock_) ? curBlock_->pop() : nullptr;
+            *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();
@@ -1395,17 +1416,17 @@ class FunctionCompiler
         MOZ_ASSERT_IF(curBlock_, !curBlock_->isMarked());
         for (uint32_t i = 0; i < join->numPredecessors(); i++)
             join->getPredecessor(i)->unmark();
 
         if (curBlock_ && !goToExistingBlock(curBlock_, join))
             return false;
         curBlock_ = join;
 
-        *def = hasPushed(curBlock_) ? curBlock_->pop() : nullptr;
+        *def = popDefIfPushed();
 
         patches.clear();
         return true;
     }
 };
 
 static bool
 EmitLiteral(FunctionCompiler& f, ValType type, MDefinition** def)
--- a/js/src/jit-test/tests/wasm/random-control-flow.js
+++ b/js/src/jit-test/tests/wasm/random-control-flow.js
@@ -123,8 +123,28 @@ wasmEvalText(`(module (func
       (i32.const 0)
      )
     )
     (br $in)
    )
   )
  )
 ))`);
+
+wasmEvalText(`
+(module
+  (func $func$0
+   (select
+    (if
+     (i32.const 0)
+     (f32.const 0)
+     (i32.const 0)
+    )
+    (if
+     (i32.const 0)
+     (f32.const 0)
+     (i32.const 0)
+    )
+    (i32.const 0)
+   )
+  )
+)
+`);