Bug 1500120 - fix compiler state for struct.narrow. r=bbouvier
authorLars T Hansen <lhansen@mozilla.com>
Thu, 18 Oct 2018 17:15:19 +0200
changeset 500484 a25a1dd8a9f58ee74fe2922c0611d3013be00091
parent 500483 fe962bfc351a0f198e3fa990693973eee5fbcf81
child 500485 ab27299789d3c74d58140d9d6651bc1102e439ed
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier
bugs1500120
milestone64.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 1500120 - fix compiler state for struct.narrow. r=bbouvier This makes sure that the compiler's view of the stack matches the run-time stack when we generate code for struct.narrow. The bug crept in here because I insisted on generating an in-line test for null before calling out to the instance. But this is unnecessary; the code in the instance can do this just fine, and null is not a common case here. (And struct.narrow is extremely slow in any case, as it's just prototype code) So, move the null check to C++ and generate straight-line code in Rabaldr.
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmInstance.cpp
js/src/wasm/WasmInstance.h
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -9234,18 +9234,25 @@ BaseCompiler::emitInstanceCall(uint32_t 
         }
         passArg(t, peek(numArgs - i), &baselineCall);
     }
     builtinInstanceMethodCall(builtin, instanceArg, baselineCall);
     endCall(baselineCall, stackSpace);
 
     popValueStackBy(numArgs);
 
-    // Note, a number of clients of emitInstanceCall currently assume that the
-    // following operation does not destroy ReturnReg.
+    // Note, many clients of emitInstanceCall currently assume that pushing the
+    // result here does not destroy ReturnReg.
+    //
+    // Furthermore, clients assume that even if retType == ExprType::Void, the
+    // callee may have returned a status result and left it in ReturnReg for us
+    // to find, and that that register will not be destroyed here (or above).
+    // In this case the callee will have a C++ declaration stating that there is
+    // a return value.  Examples include memory and table operations that are
+    // implemented as callouts.
 
     pushReturnedIfNonVoid(baselineCall, retType);
 }
 
 bool
 BaseCompiler::emitGrowMemory()
 {
     uint32_t lineOrBytecode = readCallSiteLineOrBytecode();
@@ -10075,42 +10082,31 @@ BaseCompiler::emitStructNarrow()
     }
 
     // AnyRef -> AnyRef is a no-op, just leave the value on the stack.
 
     if (inputType == ValType::AnyRef && outputType == ValType::AnyRef) {
         return true;
     }
 
-    // Null pointers are just passed through.
-
-    Label done;
-    Label doTest;
     RegPtr rp = popRef();
-    masm.branchTestPtr(Assembler::NonZero, rp, rp, &doTest);
-    pushRef(NULLREF_VALUE);
-    masm.jump(&done);
 
     // AnyRef -> (ref T) must first unbox; leaves rp or null
 
     bool mustUnboxAnyref = inputType == ValType::AnyRef;
 
     // Dynamic downcast (ref T) -> (ref U), leaves rp or null
 
     const StructType& outputStruct = env_.types[outputType.refTypeIndex()].structType();
 
-    masm.bind(&doTest);
-
     pushI32(mustUnboxAnyref);
     pushI32(outputStruct.moduleIndex_);
     pushRef(rp);
     emitInstanceCall(lineOrBytecode, SigPIIP_, ExprType::AnyRef, SymbolicAddress::StructNarrow);
 
-    masm.bind(&done);
-
     return true;
 }
 
 bool
 BaseCompiler::emitBody()
 {
     if (!iter_.readFunctionStart(funcType().ret())) {
         return false;
--- a/js/src/wasm/WasmInstance.cpp
+++ b/js/src/wasm/WasmInstance.cpp
@@ -709,23 +709,28 @@ Instance::structNew(Instance* instance, 
 {
     JSContext* cx = TlsContext.get();
     Rooted<TypeDescr*> typeDescr(cx, instance->structTypeDescrs_[typeIndex]);
     return TypedObject::createZeroed(cx, typeDescr);
 }
 
 /* static */ void*
 Instance::structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex,
-                       void* nonnullPtr)
+                       void* maybeNullPtr)
 {
     JSContext* cx = TlsContext.get();
 
     Rooted<TypedObject*> obj(cx);
     Rooted<StructTypeDescr*> typeDescr(cx);
 
+    if (maybeNullPtr == nullptr) {
+        return maybeNullPtr;
+    }
+
+    void* nonnullPtr = maybeNullPtr;
     if (mustUnboxAnyref) {
         Rooted<NativeObject*> no(cx, static_cast<NativeObject*>(nonnullPtr));
         if (!no->is<TypedObject>()) {
             return nullptr;
         }
         obj = &no->as<TypedObject>();
         Rooted<TypeDescr*> td(cx, &obj->typeDescr());
         if (td->kind() != type::Struct) {
--- a/js/src/wasm/WasmInstance.h
+++ b/js/src/wasm/WasmInstance.h
@@ -187,17 +187,18 @@ 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);
     static int32_t tableDrop(Instance* instance, uint32_t segIndex);
     static int32_t tableInit(Instance* instance, uint32_t dstOffset,
                              uint32_t srcOffset, uint32_t len, uint32_t segIndex);
     static void postBarrier(Instance* instance, gc::Cell** location);
     static void* structNew(Instance* instance, uint32_t typeIndex);
-    static void* structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex, void* ptr);
+    static void* structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex,
+                              void* maybeNullPtr);
 };
 
 typedef UniquePtr<Instance> UniqueInstance;
 
 } // namespace wasm
 } // namespace js
 
 #endif // wasm_instance_h