Bug 1593247 - Fix WebAssembly Ion codegen for if/then without else r=luke
authorAndy Wingo <wingo@igalia.com>
Mon, 04 Nov 2019 12:17:40 +0000
changeset 500335 fdca8fd9f68ec4b239ecc5e81edf76908eca86d5
parent 500334 81e027b9973731be9522e96d48a2b0b0ecbcf0b4
child 500336 fd4162c901030d6fc1da35cca16cc8a2b433a9c9
push id36763
push userrmaries@mozilla.com
push dateMon, 04 Nov 2019 21:44:06 +0000
treeherdermozilla-central@75a7a3400888 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1593247
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1593247 - Fix WebAssembly Ion codegen for if/then without else r=luke Differential Revision: https://phabricator.services.mozilla.com/D51404
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmIonCompile.cpp
js/src/wasm/WasmOpIter.h
js/src/wasm/WasmValidate.cpp
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -8778,17 +8778,17 @@ void BaseCompiler::endIfThenElse(ResultT
     pushBlockResults(type);
   }
 }
 
 bool BaseCompiler::emitEnd() {
   LabelKind kind;
   ResultType type;
   NothingVector unused_values;
-  if (!iter_.readEnd(&kind, &type, &unused_values)) {
+  if (!iter_.readEnd(&kind, &type, &unused_values, &unused_values)) {
     return false;
   }
 
   switch (kind) {
     case LabelKind::Body:
       endBlock(type);
       doReturn(ContinuationKind::Fallthrough);
       iter_.popEnd();
--- a/js/src/wasm/WasmIonCompile.cpp
+++ b/js/src/wasm/WasmIonCompile.cpp
@@ -1831,17 +1831,18 @@ static bool EmitElse(FunctionCompiler& f
 
   return true;
 }
 
 static bool EmitEnd(FunctionCompiler& f) {
   LabelKind kind;
   ResultType type;
   DefVector preJoinDefs;
-  if (!f.iter().readEnd(&kind, &type, &preJoinDefs)) {
+  DefVector resultsForEmptyElse;
+  if (!f.iter().readEnd(&kind, &type, &preJoinDefs, &resultsForEmptyElse)) {
     return false;
   }
 
   MBasicBlock* block = f.iter().controlItem();
   f.iter().popEnd();
 
   if (!f.pushDefs(preJoinDefs)) {
     return false;
@@ -1861,27 +1862,32 @@ static bool EmitEnd(FunctionCompiler& f)
         return false;
       }
       break;
     case LabelKind::Loop:
       if (!f.closeLoop(block, &postJoinDefs)) {
         return false;
       }
       break;
-    case LabelKind::Then:
+    case LabelKind::Then: {
       // If we didn't see an Else, create a trivial else block so that we create
       // a diamond anyway, to preserve Ion invariants.
       if (!f.switchToElse(block, &block)) {
         return false;
       }
 
+      if (!f.pushDefs(resultsForEmptyElse)) {
+        return false;
+      }
+
       if (!f.joinIfElse(block, &postJoinDefs)) {
         return false;
       }
       break;
+    }
     case LabelKind::Else:
       if (!f.joinIfElse(block, &postJoinDefs)) {
         return false;
       }
       break;
   }
 
   MOZ_ASSERT_IF(!f.inDeadCode(), postJoinDefs.length() == type.length());
--- a/js/src/wasm/WasmOpIter.h
+++ b/js/src/wasm/WasmOpIter.h
@@ -590,17 +590,17 @@ class MOZ_STACK_CLASS OpIter : private P
   typedef ControlStackEntry<ControlItem> Control;
   typedef Vector<Control, 8, SystemAllocPolicy> ControlStack;
 
  private:
   Decoder& d_;
   const ModuleEnvironment& env_;
 
   TypeAndValueStack valueStack_;
-  TypeAndValueStack thenParamStack_;
+  TypeAndValueStack elseParamStack_;
   ControlStack controlStack_;
 
 #ifdef DEBUG
   OpBytes op_;
 #endif
   size_t offsetOfLastReadOp_;
 
   MOZ_MUST_USE bool readFixedU8(uint8_t* out) { return d_.readFixedU8(out); }
@@ -719,19 +719,20 @@ class MOZ_STACK_CLASS OpIter : private P
   MOZ_MUST_USE bool readOp(OpBytes* op);
   MOZ_MUST_USE bool readFunctionStart(uint32_t funcIndex);
   MOZ_MUST_USE bool readFunctionEnd(const uint8_t* bodyEnd);
   MOZ_MUST_USE bool readReturn(ValueVector* values);
   MOZ_MUST_USE bool readBlock(ResultType* paramType);
   MOZ_MUST_USE bool readLoop(ResultType* paramType);
   MOZ_MUST_USE bool readIf(ResultType* paramType, Value* condition);
   MOZ_MUST_USE bool readElse(ResultType* paramType, ResultType* resultType,
-                             ValueVector* thenValues);
+                             ValueVector* thenResults);
   MOZ_MUST_USE bool readEnd(LabelKind* kind, ResultType* type,
-                            ValueVector* values);
+                            ValueVector* results,
+                            ValueVector* resultsForEmptyElse);
   void popEnd();
   MOZ_MUST_USE bool readBr(uint32_t* relativeDepth, ResultType* type,
                            ValueVector* values);
   MOZ_MUST_USE bool readBrIf(uint32_t* relativeDepth, ResultType* type,
                              ValueVector* values, Value* condition);
   MOZ_MUST_USE bool readBrTable(Uint32Vector* depths, uint32_t* defaultDepth,
                                 ResultType* defaultBranchValueType,
                                 ValueVector* branchValues, Value* index);
@@ -1216,34 +1217,34 @@ inline void OpIter<Policy>::peekOp(OpByt
     op->b0 = uint16_t(Op::Limit);
   }
 
   d_.rollbackPosition(pos);
 }
 
 template <typename Policy>
 inline bool OpIter<Policy>::readFunctionStart(uint32_t funcIndex) {
-  MOZ_ASSERT(thenParamStack_.empty());
+  MOZ_ASSERT(elseParamStack_.empty());
   MOZ_ASSERT(valueStack_.empty());
   MOZ_ASSERT(controlStack_.empty());
   MOZ_ASSERT(op_.b0 == uint16_t(Op::Limit));
   BlockType type = BlockType::FuncResults(*env_.funcTypes[funcIndex]);
   return pushControl(LabelKind::Body, type);
 }
 
 template <typename Policy>
 inline bool OpIter<Policy>::readFunctionEnd(const uint8_t* bodyEnd) {
   if (d_.currentPosition() != bodyEnd) {
     return fail("function body length mismatch");
   }
 
   if (!controlStack_.empty()) {
     return fail("unbalanced function body control flow");
   }
-  MOZ_ASSERT(thenParamStack_.empty());
+  MOZ_ASSERT(elseParamStack_.empty());
 
 #ifdef DEBUG
   op_ = OpBytes(Op::Limit);
 #endif
   valueStack_.clear();
   return true;
 }
 
@@ -1302,67 +1303,77 @@ inline bool OpIter<Policy>::readIf(Resul
   }
 
   if (!pushControl(LabelKind::Then, type)) {
     return false;
   }
 
   *paramType = type.params();
   size_t paramsLength = type.params().length();
-  return thenParamStack_.append(valueStack_.end() - paramsLength, paramsLength);
+  return elseParamStack_.append(valueStack_.end() - paramsLength, paramsLength);
 }
 
 template <typename Policy>
 inline bool OpIter<Policy>::readElse(ResultType* paramType,
                                      ResultType* resultType,
-                                     ValueVector* values) {
+                                     ValueVector* thenResults) {
   MOZ_ASSERT(Classify(op_) == OpKind::Else);
 
   Control& block = controlStack_.back();
   if (block.kind() != LabelKind::Then) {
     return fail("else can only be used within an if");
   }
 
   *paramType = block.type().params();
-  if (!checkStackAtEndOfBlock(resultType, values)) {
+  if (!checkStackAtEndOfBlock(resultType, thenResults)) {
     return false;
   }
 
-  // Restore to the entry state of the then block. Since the then block may
-  // clobbered any value in the block's params, we must restore from a
-  // snapshot.
   valueStack_.shrinkTo(block.valueStackBase());
-  size_t thenParamsLength = block.type().params().length();
-  MOZ_ASSERT(thenParamStack_.length() >= thenParamsLength);
-  valueStack_.infallibleAppend(thenParamStack_.end() - thenParamsLength,
-                               thenParamsLength);
-  thenParamStack_.shrinkBy(thenParamsLength);
+
+  size_t nparams = block.type().params().length();
+  MOZ_ASSERT(elseParamStack_.length() >= nparams);
+  valueStack_.infallibleAppend(elseParamStack_.end() - nparams, nparams);
+  elseParamStack_.shrinkBy(nparams);
 
   block.switchToElse();
   return true;
 }
 
 template <typename Policy>
 inline bool OpIter<Policy>::readEnd(LabelKind* kind, ResultType* type,
-                                    ValueVector* values) {
+                                    ValueVector* results,
+                                    ValueVector* resultsForEmptyElse) {
   MOZ_ASSERT(Classify(op_) == OpKind::End);
 
-  if (!checkStackAtEndOfBlock(type, values)) {
+  if (!checkStackAtEndOfBlock(type, results)) {
     return false;
   }
 
   Control& block = controlStack_.back();
 
-  // If an `if` block ends with `end` instead of `else`, then we must
-  // additionally validate that the then-block doesn't push anything.
   if (block.kind() == LabelKind::Then) {
-    if (block.type().params() != block.type().results()) {
+    ResultType params = block.type().params();
+    // If an `if` block ends with `end` instead of `else`, then the `else` block
+    // implicitly passes the `if` parameters as the `else` results.  In that
+    // case, assert that the `if`'s param type matches the result type.
+    if (params != block.type().results()) {
       return fail("if without else with a result value");
     }
-    thenParamStack_.shrinkBy(block.type().params().length());
+
+    size_t nparams = params.length();
+    MOZ_ASSERT(elseParamStack_.length() >= nparams);
+    if (!resultsForEmptyElse->resize(nparams)) {
+      return false;
+    }
+    const TypeAndValue* elseParams = elseParamStack_.end() - nparams;
+    for (size_t i = 0; i < nparams; i++) {
+      (*resultsForEmptyElse)[i] = elseParams[i].value();
+    }
+    elseParamStack_.shrinkBy(nparams);
   }
 
   *kind = block.kind();
   return true;
 }
 
 template <typename Policy>
 inline void OpIter<Policy>::popEnd() {
--- a/js/src/wasm/WasmValidate.cpp
+++ b/js/src/wasm/WasmValidate.cpp
@@ -483,17 +483,17 @@ static bool DecodeFunctionBodyExprs(cons
 
     Nothing nothing;
     NothingVector nothings;
     ResultType unusedType;
 
     switch (op.b0) {
       case uint16_t(Op::End): {
         LabelKind unusedKind;
-        if (!iter.readEnd(&unusedKind, &unusedType, &nothings)) {
+        if (!iter.readEnd(&unusedKind, &unusedType, &nothings, &nothings)) {
           return false;
         }
         iter.popEnd();
         if (iter.controlStackEmpty()) {
           return iter.readFunctionEnd(bodyEnd);
         }
         break;
       }