Bug 1675602 - Use ValType::isExposable in function entry and exits. r…

…=lth A previous commit added ValType::isExposable() for types that have no JS representation (currently (ref T) and v128). This commit changes stub generation to check !isExposable() instead of !isV128() so that (ref T) will be excluded. Differential Revision: https://phabricator.services.mozilla.com/D96220
dothq · Dec 13, 2020 · a44abdd · a44abdd
1 parent 79ccbdd
commit a44abdd
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 56 deletions.
diff --git a/js/src/wasm/WasmCode.cpp b/js/src/wasm/WasmCode.cpp
@@ -619,11 +619,9 @@ bool LazyStubSegment::addStubs(size_t codeLength,
     codeRanges_.back().offsetBy(offsetInSegment);
     i++;
 
-#ifdef ENABLE_WASM_SIMD
-    if (funcExports[funcExportIndex].funcType().hasV128ArgOrRet()) {
+    if (funcExports[funcExportIndex].funcType().hasUnexposableArgOrRet()) {
       continue;
     }
-#endif
     if (funcExports[funcExportIndex]
             .funcType()
             .temporarilyUnsupportedReftypeForEntry()) {
@@ -686,9 +684,7 @@ bool LazyStubTier::createMany(const Uint32Vector& funcExportIndices,
     const FuncExport& fe = funcExports[funcExportIndex];
     // Entries with unsupported types get only the interp exit
     bool unsupportedType =
-#ifdef ENABLE_WASM_SIMD
-        fe.funcType().hasV128ArgOrRet() ||
-#endif
+        fe.funcType().hasUnexposableArgOrRet() ||
         fe.funcType().temporarilyUnsupportedReftypeForEntry();
     numExpectedRanges += (unsupportedType ? 1 : 2);
     void* calleePtr =
@@ -781,9 +777,7 @@ bool LazyStubTier::createMany(const Uint32Vector& funcExportIndices,
     // Functions with unsupported types in their sig have only one entry
     // (interp).  All other functions get an extra jit entry.
     bool unsupportedType =
-#ifdef ENABLE_WASM_SIMD
-        fe.funcType().hasV128ArgOrRet() ||
-#endif
+        fe.funcType().hasUnexposableArgOrRet() ||
         fe.funcType().temporarilyUnsupportedReftypeForEntry();
     interpRangeIndex += (unsupportedType ? 1 : 2);
   }
@@ -816,14 +810,11 @@ bool LazyStubTier::createOne(uint32_t funcExportIndex,
   if (codeTier.metadata()
           .funcExports[funcExportIndex]
           .funcType()
-          .temporarilyUnsupportedReftypeForEntry()
-#ifdef ENABLE_WASM_SIMD
-      || codeTier.metadata()
-             .funcExports[funcExportIndex]
-             .funcType()
-             .hasV128ArgOrRet()
-#endif
-  ) {
+          .temporarilyUnsupportedReftypeForEntry() ||
+      codeTier.metadata()
+          .funcExports[funcExportIndex]
+          .funcType()
+          .hasUnexposableArgOrRet()) {
     MOZ_ASSERT(codeRanges.length() >= 1);
     MOZ_ASSERT(codeRanges.back().isInterpEntry());
     return true;

diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h
@@ -243,13 +243,10 @@ class FuncExport {
   }
 
   bool canHaveJitEntry() const {
-    return
-#ifdef ENABLE_WASM_SIMD
-        !funcType_.hasV128ArgOrRet() &&
-#endif
-        !funcType_.temporarilyUnsupportedReftypeForEntry() &&
-        !funcType_.temporarilyUnsupportedResultCountForJitEntry() &&
-        JitOptions.enableWasmJitEntry;
+    return !funcType_.hasUnexposableArgOrRet() &&
+           !funcType_.temporarilyUnsupportedReftypeForEntry() &&
+           !funcType_.temporarilyUnsupportedResultCountForJitEntry() &&
+           JitOptions.enableWasmJitEntry;
   }
 
   bool clone(const FuncExport& src) {

diff --git a/js/src/wasm/WasmInstance.cpp b/js/src/wasm/WasmInstance.cpp
@@ -236,13 +236,11 @@ bool Instance::callImport(JSContext* cx, uint32_t funcImportIndex,
     return false;
   }
 
-#ifdef ENABLE_WASM_SIMD
-  if (fi.funcType().hasV128ArgOrRet()) {
+  if (fi.funcType().hasUnexposableArgOrRet()) {
     JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
                              JSMSG_WASM_BAD_VAL_TYPE);
     return false;
   }
-#endif
 
   MOZ_ASSERT(argTypes.lengthWithStackResults() == argc);
   Maybe<char*> stackResultPointer;
@@ -300,10 +298,8 @@ bool Instance::callImport(JSContext* cx, uint32_t funcImportIndex,
     return true;
   }
 
-#ifdef ENABLE_WASM_SIMD
   // Should have been guarded earlier
-  MOZ_ASSERT(!fi.funcType().hasV128ArgOrRet());
-#endif
+  MOZ_ASSERT(!fi.funcType().hasUnexposableArgOrRet());
 
   // Functions with unsupported reference types in signature don't have a jit
   // exit at the moment.
@@ -1781,13 +1777,11 @@ bool Instance::callExport(JSContext* cx, uint32_t funcIndex, CallArgs args) {
     return false;
   }
 
-#ifdef ENABLE_WASM_SIMD
-  if (funcType->hasV128ArgOrRet()) {
+  if (funcType->hasUnexposableArgOrRet()) {
     JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
                              JSMSG_WASM_BAD_VAL_TYPE);
     return false;
   }
-#endif
 
   ArgTypeVector argTypes(*funcType);
   ResultType resultType(ResultType::Vector(funcType->results()));

diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp
@@ -1038,14 +1038,12 @@ static bool GenerateJitEntry(MacroAssembler& masm, size_t funcExportIndex,
 
   GenerateJitEntryLoadTls(masm, frameSize);
 
-#ifdef ENABLE_WASM_SIMD
-  if (fe.funcType().hasV128ArgOrRet()) {
+  if (fe.funcType().hasUnexposableArgOrRet()) {
     CallSymbolicAddress(masm, !fe.hasEagerStubs(),
                         SymbolicAddress::ReportV128JSCall);
     GenerateJitEntryThrow(masm, frameSize);
     return FinishOffsets(masm, offsets);
   }
-#endif
 
   FloatRegister scratchF = ABINonArgDoubleReg;
   Register scratchG = ScratchIonEntry;
@@ -1199,7 +1197,7 @@ static bool GenerateJitEntry(MacroAssembler& masm, size_t funcExportIndex,
         break;
       }
       case ValType::V128: {
-        // Guarded against by hasV128ArgOrRet()
+        // Guarded against by hasUnexposableArgOrRet()
         MOZ_CRASH("unexpected argument type when calling from the jit");
       }
       default: {
@@ -1375,7 +1373,7 @@ static bool GenerateJitEntry(MacroAssembler& masm, size_t funcExportIndex,
                                     JSReturnOperand, WasmJitEntryReturnScratch);
             break;
           case RefType::TypeIndex:
-            MOZ_CRASH("returning reference in jitentry NYI");
+            MOZ_CRASH("unexpected return type when calling from ion to wasm");
         }
         break;
       }
@@ -2876,12 +2874,10 @@ bool wasm::GenerateEntryStubs(MacroAssembler& masm, size_t funcExportIndex,
     return true;
   }
 
-#ifdef ENABLE_WASM_SIMD
   // SIMD spec requires JS calls to exports with V128 in the signature to throw.
-  if (fe.funcType().hasV128ArgOrRet()) {
+  if (fe.funcType().hasUnexposableArgOrRet()) {
     return true;
   }
-#endif
 
   // Returning multiple values to JS JIT code not yet implemented (see
   // bug 1595031).
@@ -2928,13 +2924,11 @@ bool wasm::GenerateStubs(const ModuleEnvironment& env,
       return false;
     }
 
-#ifdef ENABLE_WASM_SIMD
     // SIMD spec requires calls to JS functions with V128 in the signature to
     // throw.
-    if (fi.funcType().hasV128ArgOrRet()) {
+    if (fi.funcType().hasUnexposableArgOrRet()) {
       continue;
     }
-#endif
 
     if (fi.funcType().temporarilyUnsupportedReftypeForExit()) {
       continue;

diff --git a/js/src/wasm/WasmTypes.h b/js/src/wasm/WasmTypes.h
@@ -656,7 +656,11 @@ class ValType {
   // Returns whether the type has a representation in JS.
   bool isExposable() const {
     MOZ_ASSERT(isValid());
+#if defined(ENABLE_WASM_SIMD) || defined(ENABLE_WASM_GC)
     return kind() != ValType::V128 && !isTypeIndex();
+#else
+    return true;
+#endif
   }
 
   Kind kind() const {
@@ -1295,6 +1299,7 @@ extern MOZ_MUST_USE bool CheckEqRefValue(JSContext* cx, HandleValue v,
                                          MutableHandleAnyRef vp);
 class NoDebug;
 class DebugCodegenVal;
+
 template <typename Debug = NoDebug>
 extern bool ToWebAssemblyValue(JSContext* cx, HandleValue val, ValType type,
                                void* loc, bool mustWrite64);
@@ -1368,21 +1373,19 @@ class FuncType {
   bool temporarilyUnsupportedResultCountForJitExit() const {
     return results().length() > MaxResultsForJitExit;
   }
-#ifdef ENABLE_WASM_SIMD
-  bool hasV128ArgOrRet() const {
+  bool hasUnexposableArgOrRet() const {
     for (ValType arg : args()) {
-      if (arg == ValType::V128) {
+      if (!arg.isExposable()) {
         return true;
       }
     }
     for (ValType result : results()) {
-      if (result == ValType::V128) {
+      if (!result.isExposable()) {
         return true;
       }
     }
     return false;
   }
-#endif
   // For JS->wasm jit entries, temporarily disallow certain types until the
   // stubs generator is improved.
   //   * ref params may be nullable externrefs
@@ -1421,15 +1424,9 @@ class FuncType {
   }
   // For wasm->JS jit exits, temporarily disallow certain types until
   // the stubs generator is improved.
-  //   * ref params may not be type indices
   //   * ref results may be nullable externrefs
-  // V128 types are excluded per spec but are guarded against separately.
+  // Unexposable types must be guarded against separately.
   bool temporarilyUnsupportedReftypeForExit() const {
-    for (ValType arg : args()) {
-      if (arg.isTypeIndex()) {
-        return true;
-      }
-    }
     for (ValType result : results()) {
       if (result.isReference() &&
           (!result.isExternRef() || !result.isNullable())) {