Bug 1316332: Release stack slots in case of explicit drops; r=lth
authorBenjamin Bouvier <benj@benj.me>
Wed, 09 Nov 2016 16:29:24 +0100
changeset 322063 a67f9e4b5faf56f66f454d58118cbea584a7f334
parent 322062 07bbb0e3dde46ed12af7dc881695097cc16af045
child 322064 3930bf2158788bc3681992d61cc0d26614c8b388
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewerslth
bugs1316332
milestone52.0a1
Bug 1316332: Release stack slots in case of explicit drops; r=lth MozReview-Commit-ID: 2Jj0rzYPQ9J
js/src/jit-test/tests/wasm/drop.js
js/src/wasm/WasmBaselineCompile.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/wasm/drop.js
@@ -0,0 +1,47 @@
+load(libdir + "wasm.js");
+
+for (let type of ['i32', 'f32', 'f64']) {
+    assertEq(wasmEvalText(`
+        (module
+         (func $test (result ${type}) (param $p ${type}) (param $p2 ${type})
+            get_local $p
+            get_local $p2
+            (block)
+            drop
+         )
+         (export "test" $test)
+        )
+    `).exports.test(0x1337abc0, 0xffffffff), 0x1337abc0);
+}
+
+assertEq(wasmEvalText(`
+    (module
+     (func $test (result i32) (param $p i32) (param $p2 f32) (param $p3 f64) (param $p4 i32)
+        get_local $p
+        get_local $p2
+        get_local $p3
+        get_local $p4
+        (block)
+        drop
+        (block)
+        (block)
+        drop
+        drop
+     )
+     (export "test" $test)
+    )
+`).exports.test(0x1337abc0, 0xffffffff), 0x1337abc0);
+
+setJitCompilerOption('wasm.test-mode', 1);
+
+assertEqI64(wasmEvalText(`
+    (module
+     (func $test (result i64) (param $p i64) (param $p2 i64)
+        get_local $p
+        get_local $p2
+        (block)
+        drop
+     )
+     (export "test" $test)
+    )
+`).exports.test(createI64(0x1337abc0), createI64(0xffffffff | 0)), createI64(0x1337abc0));
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -892,27 +892,28 @@ class BaseCompiler
             RawF64   f64val_;
             uint32_t slot_;
             uint32_t offs_;
         };
 
         Stk() { kind_ = None; }
 
         Kind kind() const { return kind_; }
+        bool isMem() const { return kind_ <= MemLast; }
 
         RegI32   i32reg() const { MOZ_ASSERT(kind_ == RegisterI32); return i32reg_; }
         RegI64   i64reg() const { MOZ_ASSERT(kind_ == RegisterI64); return i64reg_; }
         RegF32   f32reg() const { MOZ_ASSERT(kind_ == RegisterF32); return f32reg_; }
         RegF64   f64reg() const { MOZ_ASSERT(kind_ == RegisterF64); return f64reg_; }
         int32_t  i32val() const { MOZ_ASSERT(kind_ == ConstI32); return i32val_; }
         int64_t  i64val() const { MOZ_ASSERT(kind_ == ConstI64); return i64val_; }
         RawF32   f32val() const { MOZ_ASSERT(kind_ == ConstF32); return f32val_; }
         RawF64   f64val() const { MOZ_ASSERT(kind_ == ConstF64); return f64val_; }
         uint32_t slot() const { MOZ_ASSERT(kind_ > MemLast && kind_ <= LocalLast); return slot_; }
-        uint32_t offs() const { MOZ_ASSERT(kind_ <= MemLast); return offs_; }
+        uint32_t offs() const { MOZ_ASSERT(isMem()); return offs_; }
 
         void setI32Reg(RegI32 r) { kind_ = RegisterI32; i32reg_ = r; }
         void setI64Reg(RegI64 r) { kind_ = RegisterI64; i64reg_ = r; }
         void setF32Reg(RegF32 r) { kind_ = RegisterF32; f32reg_ = r; }
         void setF64Reg(RegF64 r) { kind_ = RegisterF64; f64reg_ = r; }
         void setI32Val(int32_t v) { kind_ = ConstI32; i32val_ = v; }
         void setI64Val(int64_t v) { kind_ = ConstI64; i64val_ = v; }
         void setF32Val(RawF32 v) { kind_ = ConstF32; f32val_ = v; }
@@ -1268,17 +1269,17 @@ class BaseCompiler
     //    sync.  That type of fix would go into needI32().
 
     void sync() {
         size_t start = 0;
         size_t lim = stk_.length();
 
         for (size_t i = lim; i > 0; i--) {
             // Memory opcodes are first in the enum, single check against MemLast is fine.
-            if (stk_[i-1].kind() <= Stk::MemLast) {
+            if (stk_[i - 1].kind() <= Stk::MemLast) {
                 start = i;
                 break;
             }
         }
 
         for (size_t i = start; i < lim; i++) {
             Stk& v = stk_[i];
             switch (v.kind()) {
@@ -1802,17 +1803,17 @@ class BaseCompiler
     }
 
     // Return the amount of execution stack consumed by the top numval
     // values on the value stack.
 
     size_t stackConsumed(size_t numval) {
         size_t size = 0;
         MOZ_ASSERT(numval <= stk_.length());
-        for (uint32_t i = stk_.length()-1; numval > 0; numval--, i--) {
+        for (uint32_t i = stk_.length() - 1; numval > 0; numval--, i--) {
             // The size computations come from the implementation of Push() in
             // MacroAssembler-x86-shared.cpp and MacroAssembler-arm-shared.cpp,
             // and from VFPRegister::size() in Architecture-arm.h.
             //
             // On ARM unlike on x86 we push a single for float.
 
             Stk& v = stk_[i];
             switch (v.kind()) {
@@ -1899,16 +1900,21 @@ class BaseCompiler
         if (frameHere > framePushed) {
             if (deadCode_)
                 masm.adjustStack(frameHere - framePushed);
             else
                 masm.freeStack(frameHere - framePushed);
         }
     }
 
+    void popStackIfMemory() {
+        if (peek(0).isMem())
+            masm.freeStack(stackConsumed(1));
+    }
+
     // Peek at the stack, for calls.
 
     Stk& peek(uint32_t relativeDepth) {
         return stk_[stk_.length()-1-relativeDepth];
     }
 
     ////////////////////////////////////////////////////////////
     //
@@ -2028,34 +2034,35 @@ class BaseCompiler
         // systems we have 512-bit SIMD for that matter.)
         //
         // TODO / OPTIMIZE: if we have only one initializing store
         // then it's better to store a zero literal, probably.
 
         if (varLow_ < varHigh_) {
             ScratchI32 scratch(*this);
             masm.mov(ImmWord(0), scratch);
-            for (int32_t i = varLow_ ; i < varHigh_ ; i+=4)
-                storeToFrameI32(scratch, i+4);
+            for (int32_t i = varLow_ ; i < varHigh_ ; i += 4)
+                storeToFrameI32(scratch, i + 4);
         }
     }
 
     bool endFunction() {
         // Out-of-line prologue.  Assumes that the in-line prologue has
         // been executed and that a frame of size = localSize_ + sizeof(Frame)
         // has been allocated.
 
         masm.bind(&outOfLinePrologue_);
 
         MOZ_ASSERT(maxFramePushed_ >= localSize_);
 
         // ABINonArgReg0 != ScratchReg, which can be used by branchPtr().
 
         masm.movePtr(masm.getStackPointer(), ABINonArgReg0);
-        masm.subPtr(Imm32(maxFramePushed_ - localSize_), ABINonArgReg0);
+        if (maxFramePushed_ - localSize_)
+            masm.subPtr(Imm32(maxFramePushed_ - localSize_), ABINonArgReg0);
         masm.branchPtr(Assembler::Below,
                        Address(WasmTlsReg, offsetof(TlsData, stackLimit)),
                        ABINonArgReg0,
                        &bodyLabel_);
 
         // Since we just overflowed the stack, to be on the safe side, pop the
         // stack so that, when the trap exit stub executes, it is a safe
         // distance away from the end of the native stack.
@@ -3575,16 +3582,17 @@ class BaseCompiler
     MOZ_MUST_USE bool emitBlock();
     MOZ_MUST_USE bool emitLoop();
     MOZ_MUST_USE bool emitIf();
     MOZ_MUST_USE bool emitElse();
     MOZ_MUST_USE bool emitEnd();
     MOZ_MUST_USE bool emitBr();
     MOZ_MUST_USE bool emitBrIf();
     MOZ_MUST_USE bool emitBrTable();
+    MOZ_MUST_USE bool emitDrop();
     MOZ_MUST_USE bool emitReturn();
     MOZ_MUST_USE bool emitCallArgs(const ValTypeVector& args, FunctionCall& baselineCall);
     MOZ_MUST_USE bool emitCall();
     MOZ_MUST_USE bool emitCallIndirect(bool oldStyle);
     MOZ_MUST_USE bool emitCommonMathCall(uint32_t lineOrBytecode, SymbolicAddress callee,
                                          ValTypeVector& signature, ExprType retType);
     MOZ_MUST_USE bool emitUnaryMathBuiltinCall(SymbolicAddress callee, ValType operandType);
     MOZ_MUST_USE bool emitBinaryMathBuiltinCall(SymbolicAddress callee, ValType operandType);
@@ -5194,16 +5202,30 @@ BaseCompiler::emitBrTable()
     for (uint32_t i = 0; i < tableLength; i++)
         freeLabel(stubs[i]);
 
     popValueStackTo(ctl_.back().stackSize);
 
     return true;
 }
 
+bool
+BaseCompiler::emitDrop()
+{
+    if (!iter_.readDrop())
+        return false;
+
+    if (deadCode_)
+        return true;
+
+    popStackIfMemory();
+    popValueStackBy(1);
+    return true;
+}
+
 void
 BaseCompiler::doReturn(ExprType type)
 {
     switch (type) {
       case ExprType::Void: {
         returnCleanup();
         break;
       }
@@ -6552,20 +6574,17 @@ BaseCompiler::emitBody()
         CHECK(iter_.readExpr(&expr));
 
         switch (expr) {
           // Control opcodes
           case Expr::Nop:
             CHECK(iter_.readNop());
             NEXT();
           case Expr::Drop:
-            CHECK(iter_.readDrop());
-            if (!deadCode_)
-                popValueStackBy(1);
-            NEXT();
+            CHECK_NEXT(emitDrop());
           case Expr::Block:
             CHECK_NEXT(emitBlock());
           case Expr::Loop:
             CHECK_NEXT(emitLoop());
           case Expr::If:
             CHECK_NEXT(emitIf());
           case Expr::Else:
             CHECK_NEXT(emitElse());