Bug 1541357 - Change the argument order of table.grow to be spec-compliant. r=lth
authorJulian Seward <jseward@acm.org>
Mon, 22 Apr 2019 08:51:04 +0000
changeset 470441 74175527c5eab80b079b60a97b227e5ec5721c66
parent 470440 0b0a2b12bd0502a0743917abdd29109c2b2cdbfd
child 470442 02a9bf38ebf276bcd4950a7ff6286e8688da15b3
push id35906
push useraciure@mozilla.com
push dateTue, 23 Apr 2019 22:14:56 +0000
treeherdermozilla-central@0ce3633f8b80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1541357
milestone68.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 1541357 - Change the argument order of table.grow to be spec-compliant. r=lth We implemented table.grow with an initializer argument before there was a spec for it, and the draft spec now takes the arguments in the opposite order of what we have, to follow table.fill and memory.fill: we should pop the delta first, then the init value, ie in the high-level notation, the init value is the first argument and the delta the second. This corresponds to the last two arguments of the fill instructions, which are initializer value and length. This commit fixes both the implementation and test cases: it swaps the order of the 'initial value' and 'delta' arguments. Differential Revision: https://phabricator.services.mozilla.com/D27887
js/src/jit-test/tests/wasm/gc/tables-generalized-struct.js
js/src/jit-test/tests/wasm/gc/tables-generalized.js
js/src/jit-test/tests/wasm/gc/tables-multiple.js
js/src/jit-test/tests/wasm/gc/tables-stress.js
js/src/wasm/WasmAST.h
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmBuiltins.cpp
js/src/wasm/WasmInstance.cpp
js/src/wasm/WasmInstance.h
js/src/wasm/WasmIonCompile.cpp
js/src/wasm/WasmOpIter.h
js/src/wasm/WasmTextToBinary.cpp
--- a/js/src/jit-test/tests/wasm/gc/tables-generalized-struct.js
+++ b/js/src/jit-test/tests/wasm/gc/tables-generalized-struct.js
@@ -36,15 +36,15 @@
 
 {
     let ins = wasmEvalText(
         `(module
           (gc_feature_opt_in 3)
           (type $S (struct (field i32) (field f64)))
           (table (export "t") 2 anyref)
           (func (export "f") (result i32)
-           (table.grow (i32.const 1) (struct.new $S (i32.const 0) (f64.const 3.14)))))`);
+           (table.grow (struct.new $S (i32.const 0) (f64.const 3.14)) (i32.const 1))))`);
     assertEq(ins.exports.t.length, 2);
     assertEq(ins.exports.f(), 2);
     assertEq(ins.exports.t.length, 3);
     assertEq(typeof ins.exports.t.get(2), "object");
 }
 
--- a/js/src/jit-test/tests/wasm/gc/tables-generalized.js
+++ b/js/src/jit-test/tests/wasm/gc/tables-generalized.js
@@ -275,17 +275,17 @@ assertErrorMessage(() => new WebAssembly
 // table.grow with delta - works and returns correct old value
 // table.grow with delta at upper limit - fails
 // table.grow with negative delta - fails
 
 let ins = wasmEvalText(
     `(module
       (table (export "t") 10 20 anyref)
       (func (export "grow") (param i32) (result i32)
-       (table.grow (local.get 0) (ref.null))))`);
+       (table.grow (ref.null) (local.get 0))))`);
 assertEq(ins.exports.grow(0), 10);
 assertEq(ins.exports.t.length, 10);
 assertEq(ins.exports.grow(1), 10);
 assertEq(ins.exports.t.length, 11);
 assertEq(ins.exports.t.get(10), null);
 assertEq(ins.exports.grow(9), 11);
 assertEq(ins.exports.t.length, 20);
 assertEq(ins.exports.t.get(19), null);
@@ -303,63 +303,63 @@ assertEq(ins.exports.t.length, 20)
 
 // Special case for private tables without a maximum
 
 {
     let ins = wasmEvalText(
         `(module
           (table 10 anyref)
           (func (export "grow") (param i32) (result i32)
-           (table.grow (local.get 0) (ref.null))))`);
+           (table.grow (ref.null) (local.get 0))))`);
     assertEq(ins.exports.grow(0), 10);
     assertEq(ins.exports.grow(1), 10);
     assertEq(ins.exports.grow(9), 11);
     assertEq(ins.exports.grow(0), 20);
 }
 
 // Can't grow table of funcref yet
 
 assertErrorMessage(() => wasmEvalText(
     `(module
       (table $t 2 funcref)
       (func $f
-       (drop (table.grow (i32.const 1) (ref.null)))))`),
+       (drop (table.grow (ref.null) (i32.const 1)))))`),
                    WebAssembly.CompileError,
                    /table.grow only on tables of anyref/);
 
 // table.grow with non-i32 argument - fails validation
 
 assertErrorMessage(() => new WebAssembly.Module(wasmTextToBinary(
     `(module
        (table 10 anyref)
        (func (export "f") (param f64)
-        (table.grow (local.get 0) (ref.null))))`)),
+        (table.grow (ref.null) (local.get 0))))`)),
                    WebAssembly.CompileError,
                    /type mismatch/);
 
 // table.grow when there are no tables - fails validation
 
 assertErrorMessage(() => new WebAssembly.Module(wasmTextToBinary(
     `(module
        (func (export "f") (param i32)
-        (table.grow (local.get 0) (ref.null))))`)),
+        (table.grow (ref.null) (local.get 0))))`)),
                    WebAssembly.CompileError,
                    /table index out of range for table.grow/);
 
 // table.size on table of anyref
 
 for (let visibility of ['', '(export "t")', '(import "m" "t")']) {
     let exp = {m:{t: new WebAssembly.Table({element:"anyref",
                                             initial: 10,
                                             maximum: 20})}};
     let ins = wasmEvalText(
         `(module
           (table ${visibility} 10 20 anyref)
           (func (export "grow") (param i32) (result i32)
-           (table.grow (local.get 0) (ref.null)))
+           (table.grow (ref.null) (local.get 0)))
           (func (export "size") (result i32)
            (table.size)))`,
         exp);
     assertEq(ins.exports.grow(0), 10);
     assertEq(ins.exports.size(), 10);
     assertEq(ins.exports.grow(1), 10);
     assertEq(ins.exports.size(), 11);
     assertEq(ins.exports.grow(9), 11);
--- a/js/src/jit-test/tests/wasm/gc/tables-multiple.js
+++ b/js/src/jit-test/tests/wasm/gc/tables-multiple.js
@@ -107,17 +107,17 @@ assertEq(exp.m.t3.get(4), x);
 
 var exp = {m:{t0: new WebAssembly.Table({element:"anyref", initial:2}),
               t1: new WebAssembly.Table({element:"anyref", initial:3})}};
 var ins = wasmEvalText(
     `(module
       (table $t0 (import "m" "t0") 2 anyref)
       (table $t1 (import "m" "t1") 3 anyref)
       (func (export "f") (result i32)
-       (table.grow $t1 (i32.const 5) (ref.null)))
+       (table.grow $t1 (ref.null) (i32.const 5)))
       (func (export "size0") (result i32)
        (table.size $t0))
       (func (export "size1") (result i32)
        (table.size $t1)))`,
     exp);
 
 assertEq(ins.exports.f(), 3);
 assertEq(exp.m.t1.length, 8);
@@ -218,19 +218,19 @@ var tbl = new WebAssembly.Table({element
 var exp = {m: {t0: tbl, t1:tbl}};
 
 var ins = wasmEvalText(
     `(module
       (import $t0 "m" "t0" (table 1 anyref))
       (import $t1 "m" "t1" (table 1 anyref))
       (table $t2 (export "t2") 1 funcref)
       (func (export "f") (result i32)
-       (table.grow $t0 (i32.const 1) (ref.null)))
+       (table.grow $t0 (ref.null) (i32.const 1)))
       (func (export "g") (result i32)
-       (table.grow $t1 (i32.const 1) (ref.null)))
+       (table.grow $t1 (ref.null) (i32.const 1)))
       (func (export "size") (result i32)
        (table.size $t2)))`,
     exp);
 
 assertEq(ins.exports.f(), 1);
 assertEq(ins.exports.g(), 2);
 assertEq(ins.exports.f(), 3);
 assertEq(ins.exports.g(), 4);
@@ -364,17 +364,17 @@ assertErrorMessage(() => wasmEvalText(
                    WebAssembly.CompileError,
                    /table index out of range for table.size/);
 
 assertErrorMessage(() => wasmEvalText(
     `(module
       (table $t0 2 anyref)
       (table $t1 2 anyref)
       (func $f (result i32)
-       (table.grow 2 (i32.const 1) (ref.null))))`),
+       (table.grow 2 (ref.null) (i32.const 1))))`),
                    WebAssembly.CompileError,
                    /table index out of range for table.grow/);
 
 assertErrorMessage(() => wasmEvalText(
     `(module
       (table $t0 2 funcref)
       (elem passive) ;; 0
       (func $f (result i32)
@@ -443,17 +443,17 @@ wasmEvalText(
          (table.get (get_local 0))
          (drop)
         )
     )`);
 
 wasmEvalText(
     `(module
        (table (export "t") 10 anyref)
-       (func (param i32) (param anyref)
+       (func (param i32) (param i32)
          (return)
          (table.grow (get_local 1))
          (drop)
         )
     )`);
 
 wasmEvalText(
     `(module
--- a/js/src/jit-test/tests/wasm/gc/tables-stress.js
+++ b/js/src/jit-test/tests/wasm/gc/tables-stress.js
@@ -7,17 +7,17 @@ for ( let prefix of ['', '(table $prefix
        (table $tbl 0 anyref)
        (import $item "m" "item" (func (result anyref)))
        (func (export "run") (param $numiters i32)
          (local $i i32)
          (local $j i32)
          (local $last i32)
          (local $iters i32)
          (local $tmp anyref)
-         (local.set $last (table.grow $tbl (i32.const 1) (ref.null)))
+         (local.set $last (table.grow $tbl (ref.null) (i32.const 1)))
          (table.set $tbl (local.get $last) (call $item))
          (loop $iter_continue
            (local.set $i (i32.const 0))
            (local.set $j (local.get $last))
            (block $l_break
              (loop $l_continue
                (br_if $l_break (i32.ge_s (local.get $j) (local.get $i)))
                (local.set $tmp (table.get $tbl (local.get $i)))
--- a/js/src/wasm/WasmAST.h
+++ b/js/src/wasm/WasmAST.h
@@ -931,30 +931,30 @@ class AstTableGet : public AstExpr {
         index_(index) {}
 
   AstRef& targetTable() { return targetTable_; }
   AstExpr& index() const { return *index_; }
 };
 
 class AstTableGrow : public AstExpr {
   AstRef targetTable_;
+  AstExpr* initValue_;
   AstExpr* delta_;
-  AstExpr* initValue_;
 
  public:
   static const AstExprKind Kind = AstExprKind::TableGrow;
-  AstTableGrow(AstRef targetTable, AstExpr* delta, AstExpr* initValue)
+  AstTableGrow(AstRef targetTable, AstExpr* initValue, AstExpr* delta)
       : AstExpr(Kind, ExprType::I32),
         targetTable_(targetTable),
-        delta_(delta),
-        initValue_(initValue) {}
+        initValue_(initValue),
+        delta_(delta) {}
 
   AstRef& targetTable() { return targetTable_; }
+  AstExpr& initValue() const { return *initValue_; }
   AstExpr& delta() const { return *delta_; }
-  AstExpr& initValue() const { return *initValue_; }
 };
 
 class AstTableSet : public AstExpr {
   AstRef targetTable_;
   AstExpr* index_;
   AstExpr* value_;
 
  public:
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -10355,23 +10355,23 @@ bool BaseCompiler::emitTableGet() {
 }
 
 MOZ_MUST_USE
 bool BaseCompiler::emitTableGrow() {
   uint32_t lineOrBytecode = readCallSiteLineOrBytecode();
   Nothing delta;
   Nothing initValue;
   uint32_t tableIndex;
-  if (!iter_.readTableGrow(&tableIndex, &delta, &initValue)) {
+  if (!iter_.readTableGrow(&tableIndex, &initValue, &delta)) {
     return false;
   }
   if (deadCode_) {
     return true;
   }
-  // grow(delta:u32, initValue:anyref, table:u32) -> u32
+  // grow(initValue:anyref, delta:u32, table:u32) -> u32
   //
   // infallible.
   pushI32(tableIndex);
   return emitInstanceCall(lineOrBytecode, SASigTableGrow);
 }
 
 MOZ_MUST_USE
 bool BaseCompiler::emitTableSet() {
--- a/js/src/wasm/WasmBuiltins.cpp
+++ b/js/src/wasm/WasmBuiltins.cpp
@@ -119,17 +119,17 @@ const SymbolicAddressSignature SASigTabl
     _I32,
     6,
     {_PTR, _I32, _I32, _I32, _I32, _I32, _END}};
 const SymbolicAddressSignature SASigElemDrop = {
     SymbolicAddress::ElemDrop, _I32, 2, {_PTR, _I32, _END}};
 const SymbolicAddressSignature SASigTableGet = {
     SymbolicAddress::TableGet, _PTR, 3, {_PTR, _I32, _I32, _END}};
 const SymbolicAddressSignature SASigTableGrow = {
-    SymbolicAddress::TableGrow, _I32, 4, {_PTR, _I32, _RoN, _I32, _END}};
+    SymbolicAddress::TableGrow, _I32, 4, {_PTR, _RoN, _I32, _I32, _END}};
 const SymbolicAddressSignature SASigTableInit = {
     SymbolicAddress::TableInit,
     _I32,
     6,
     {_PTR, _I32, _I32, _I32, _I32, _I32, _END}};
 const SymbolicAddressSignature SASigTableSet = {
     SymbolicAddress::TableSet, _I32, 4, {_PTR, _I32, _RoN, _I32, _END}};
 const SymbolicAddressSignature SASigTableSize = {
--- a/js/src/wasm/WasmInstance.cpp
+++ b/js/src/wasm/WasmInstance.cpp
@@ -869,17 +869,17 @@ Instance::tableGet(Instance* instance, u
     JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
                               JSMSG_WASM_TABLE_OUT_OF_BOUNDS);
     return nullptr;
   }
   return const_cast<void*>(table.getShortlivedAnyRefLocForCompiledCode(index));
 }
 
 /* static */ uint32_t /* infallible */
-Instance::tableGrow(Instance* instance, uint32_t delta, void* initValue,
+Instance::tableGrow(Instance* instance, void* initValue, uint32_t delta,
                     uint32_t tableIndex) {
   RootedAnyRef obj(TlsContext.get(), AnyRef::fromCompiledCode(initValue));
   Table& table = *instance->tables()[tableIndex];
   MOZ_RELEASE_ASSERT(table.kind() == TableKind::AnyRef);
 
   uint32_t oldSize = table.grow(delta, TlsContext.get());
   if (oldSize != uint32_t(-1) && initValue != nullptr) {
     for (uint32_t i = 0; i < delta; i++) {
--- a/js/src/wasm/WasmInstance.h
+++ b/js/src/wasm/WasmInstance.h
@@ -197,17 +197,17 @@ class Instance {
   static int32_t memInit(Instance* instance, uint32_t dstOffset,
                          uint32_t srcOffset, uint32_t len, uint32_t segIndex);
   static int32_t tableCopy(Instance* instance, uint32_t dstOffset,
                            uint32_t srcOffset, uint32_t len,
                            uint32_t dstTableIndex, uint32_t srcTableIndex);
   static int32_t elemDrop(Instance* instance, uint32_t segIndex);
   static void* tableGet(Instance* instance, uint32_t index,
                         uint32_t tableIndex);
-  static uint32_t tableGrow(Instance* instance, uint32_t delta, void* initValue,
+  static uint32_t tableGrow(Instance* instance, void* initValue, uint32_t delta,
                             uint32_t tableIndex);
   static int32_t tableSet(Instance* instance, uint32_t index, void* value,
                           uint32_t tableIndex);
   static uint32_t tableSize(Instance* instance, uint32_t tableIndex);
   static int32_t tableInit(Instance* instance, uint32_t dstOffset,
                            uint32_t srcOffset, uint32_t len, uint32_t segIndex,
                            uint32_t tableIndex);
   static void postBarrier(Instance* instance, gc::Cell** location);
--- a/js/src/wasm/WasmIonCompile.cpp
+++ b/js/src/wasm/WasmIonCompile.cpp
@@ -3189,39 +3189,39 @@ static bool EmitTableGet(FunctionCompile
   }
 
   f.iter().setResult(ret);
   return true;
 }
 
 static bool EmitTableGrow(FunctionCompiler& f) {
   uint32_t tableIndex;
+  MDefinition* initValue;
   MDefinition* delta;
-  MDefinition* initValue;
-  if (!f.iter().readTableGrow(&tableIndex, &delta, &initValue)) {
+  if (!f.iter().readTableGrow(&tableIndex, &initValue, &delta)) {
     return false;
   }
 
   if (f.inDeadCode()) {
     return true;
   }
 
   uint32_t lineOrBytecode = f.readCallSiteLineOrBytecode();
 
   const SymbolicAddressSignature& callee = SASigTableGrow;
   CallCompileState args;
   if (!f.passInstance(callee.argTypes[0], &args)) {
     return false;
   }
 
-  if (!f.passArg(delta, callee.argTypes[1], &args)) {
+  if (!f.passArg(initValue, callee.argTypes[1], &args)) {
     return false;
   }
 
-  if (!f.passArg(initValue, callee.argTypes[2], &args)) {
+  if (!f.passArg(delta, callee.argTypes[2], &args)) {
     return false;
   }
 
   MDefinition* tableIndexArg =
       f.constant(Int32Value(tableIndex), MIRType::Int32);
   if (!tableIndexArg) {
     return false;
   }
--- a/js/src/wasm/WasmOpIter.h
+++ b/js/src/wasm/WasmOpIter.h
@@ -462,18 +462,18 @@ class MOZ_STACK_CLASS OpIter : private P
                                        Value* dst, uint32_t* srcMemOrTableIndex,
                                        Value* src, Value* len);
   MOZ_MUST_USE bool readDataOrElemDrop(bool isData, uint32_t* segIndex);
   MOZ_MUST_USE bool readMemFill(Value* start, Value* val, Value* len);
   MOZ_MUST_USE bool readMemOrTableInit(bool isMem, uint32_t* segIndex,
                                        uint32_t* dstTableIndex, Value* dst,
                                        Value* src, Value* len);
   MOZ_MUST_USE bool readTableGet(uint32_t* tableIndex, Value* index);
-  MOZ_MUST_USE bool readTableGrow(uint32_t* tableIndex, Value* delta,
-                                  Value* initValue);
+  MOZ_MUST_USE bool readTableGrow(uint32_t* tableIndex, Value* initValue,
+                                  Value* delta);
   MOZ_MUST_USE bool readTableSet(uint32_t* tableIndex, Value* index,
                                  Value* value);
   MOZ_MUST_USE bool readTableSize(uint32_t* tableIndex);
   MOZ_MUST_USE bool readStructNew(uint32_t* typeIndex, ValueVector* argValues);
   MOZ_MUST_USE bool readStructGet(uint32_t* typeIndex, uint32_t* fieldIndex,
                                   Value* ptr);
   MOZ_MUST_USE bool readStructSet(uint32_t* typeIndex, uint32_t* fieldIndex,
                                   Value* ptr, Value* val);
@@ -1993,24 +1993,24 @@ inline bool OpIter<Policy>::readTableGet
     return fail("table.get only on tables of anyref");
   }
 
   infalliblePush(ValType::AnyRef);
   return true;
 }
 
 template <typename Policy>
-inline bool OpIter<Policy>::readTableGrow(uint32_t* tableIndex, Value* delta,
-                                          Value* initValue) {
+inline bool OpIter<Policy>::readTableGrow(uint32_t* tableIndex, Value* initValue,
+                                          Value* delta) {
   MOZ_ASSERT(Classify(op_) == OpKind::TableGrow);
 
-  if (!popWithType(ValType::AnyRef, initValue)) {
+  if (!popWithType(ValType::I32, delta)) {
     return false;
   }
-  if (!popWithType(ValType::I32, delta)) {
+  if (!popWithType(ValType::AnyRef, initValue)) {
     return false;
   }
 
   if (!readVarU32(tableIndex)) {
     return false;
   }
   if (*tableIndex >= env_.tables.length()) {
     return fail("table index out of range for table.grow");
--- a/js/src/wasm/WasmTextToBinary.cpp
+++ b/js/src/wasm/WasmTextToBinary.cpp
@@ -3801,33 +3801,33 @@ static AstTableGet* ParseTableGet(WasmPa
   AstExpr* index = ParseExpr(c, inParens);
   if (!index) {
     return nullptr;
   }
   return new (c.lifo) AstTableGet(targetTable, index);
 }
 
 static AstTableGrow* ParseTableGrow(WasmParseContext& c, bool inParens) {
-  // (table.grow table delta)
-  // (table.grow delta)
+  // (table.grow table initValue delta)
+  // (table.grow initValue delta)
 
   AstRef targetTable = AstRef(0);
   c.ts.getIfRef(&targetTable);
 
+  AstExpr* initValue = ParseExpr(c, inParens);
+  if (!initValue) {
+    return nullptr;
+  }
+
   AstExpr* delta = ParseExpr(c, inParens);
   if (!delta) {
     return nullptr;
   }
 
-  AstExpr* initValue = ParseExpr(c, inParens);
-  if (!initValue) {
-    return nullptr;
-  }
-
-  return new (c.lifo) AstTableGrow(targetTable, delta, initValue);
+  return new (c.lifo) AstTableGrow(targetTable, initValue, delta);
 }
 
 static AstTableSet* ParseTableSet(WasmParseContext& c, bool inParens) {
   // (table.set table index value)
   // (table.set index value)
 
   AstRef targetTable = AstRef(0);
   c.ts.getIfRef(&targetTable);
@@ -6434,17 +6434,17 @@ static bool EncodeMemOrTableInit(Encoder
 
 #ifdef ENABLE_WASM_REFTYPES
 static bool EncodeTableGet(Encoder& e, AstTableGet& s) {
   return EncodeExpr(e, s.index()) && e.writeOp(Op::TableGet) &&
          e.writeVarU32(s.targetTable().index());
 }
 
 static bool EncodeTableGrow(Encoder& e, AstTableGrow& s) {
-  return EncodeExpr(e, s.delta()) && EncodeExpr(e, s.initValue()) &&
+  return EncodeExpr(e, s.initValue()) && EncodeExpr(e, s.delta()) &&
          e.writeOp(MiscOp::TableGrow) && e.writeVarU32(s.targetTable().index());
 }
 
 static bool EncodeTableSet(Encoder& e, AstTableSet& s) {
   return EncodeExpr(e, s.index()) && EncodeExpr(e, s.value()) &&
          e.writeOp(Op::TableSet) && e.writeVarU32(s.targetTable().index());
 }