Bug 1234985 - Odin: hoist read/writeOp into wasm::Decoder/Encoder (r=bbouvier)

js/src/asmjs/AsmJS.cpp

@@ -2714,8 +2714,7 @@ class MOZ_STACK_CLASS FunctionValidator
 
     MOZ_WARN_UNUSED_RESULT
     bool writeOp(Expr op) {
-        static_assert(sizeof(Expr) == sizeof(uint8_t), "opcodes must be uint8");
-        return encoder().writeU8(uint8_t(op));
+        return encoder().writeExpr(op);
     }
 
     MOZ_WARN_UNUSED_RESULT
@@ -2772,8 +2771,7 @@ class MOZ_STACK_CLASS FunctionValidator
     }
 
     void patchOp(size_t pos, Expr stmt) {
-        static_assert(sizeof(Expr) == sizeof(uint8_t), "opcodes must be uint8");
-        encoder().patchU8(pos, uint8_t(stmt));
+        encoder().patchExpr(pos, stmt);
     }
     void patchU8(size_t pos, uint8_t u8) {
         encoder().patchU8(pos, u8);
@@ -2790,7 +2788,7 @@ class MOZ_STACK_CLASS FunctionValidator
     }
     MOZ_WARN_UNUSED_RESULT
     bool tempOp(size_t* offset) {
-        return tempU8(offset);
+        return encoder().writeExpr(Expr::Unreachable, offset);
     }
     MOZ_WARN_UNUSED_RESULT
     bool temp32(size_t* offset) {

js/src/asmjs/WasmBinary.h

@@ -311,7 +311,7 @@ class Encoder
     UniqueBytecode bytecode_;
     DebugOnly<bool> done_;
 
-    template<class T>
+    template <class T>
     MOZ_WARN_UNUSED_RESULT
     bool write(T v, size_t* offset) {
         if (offset)
@@ -319,6 +319,26 @@ class Encoder
         return bytecode_->append(reinterpret_cast<uint8_t*>(&v), sizeof(T));
     }
 
+    template <class T>
+    MOZ_WARN_UNUSED_RESULT
+    bool writeEnum(T v, size_t* offset) {
+        // For now, just write a u8 instead of a variable-length integer.
+        // Variable-length is somewhat annoying at the moment due to the
+        // pre-order encoding and back-patching; let's see if we switch to
+        // post-order first.
+        static_assert(mozilla::IsEnum<T>::value, "is an enum");
+        MOZ_ASSERT(uint64_t(v) < UINT8_MAX);
+        return writeU8(uint8_t(v), offset);
+    }
+
+    template <class T>
+    void patchEnum(size_t pc, T v) {
+        // See writeEnum comment.
+        static_assert(mozilla::IsEnum<T>::value, "is an enum");
+        MOZ_ASSERT(uint64_t(v) < UINT8_MAX);
+        (*bytecode_)[pc] = uint8_t(v);
+    }
+
   public:
     Encoder()
       : bytecode_(nullptr),
@@ -357,6 +377,11 @@ class Encoder
         return true;
     }
 
+    MOZ_WARN_UNUSED_RESULT bool
+    writeExpr(Expr expr, size_t* offset = nullptr) {
+        return writeEnum(expr, offset);
+    }
+
     MOZ_WARN_UNUSED_RESULT bool
     writeU8(uint8_t i, size_t* offset = nullptr)   { return write<uint8_t>(i, offset); }
     MOZ_WARN_UNUSED_RESULT bool
@@ -397,12 +422,14 @@ class Encoder
         return patchable;
     }
 #endif
-
     void patchU8(size_t pc, uint8_t i) {
         MOZ_ASSERT(pcIsPatchable(pc, sizeof(uint8_t)));
         (*bytecode_)[pc] = i;
     }
-
+    void patchExpr(size_t pc, Expr expr) {
+        MOZ_ASSERT(pcIsPatchable(pc, sizeof(uint8_t)));
+        patchEnum(pc, expr);
+    }
     template<class T>
     void patch32(size_t pc, T i) {
         static_assert(sizeof(T) == sizeof(uint32_t),
@@ -418,7 +445,7 @@ class Decoder
     const uint8_t* const end_;
     const uint8_t* cur_;
 
-    template<class T>
+    template <class T>
     MOZ_WARN_UNUSED_RESULT bool
     read(T* out) {
         if (uintptr_t(end_ - cur_) < sizeof(T))
@@ -428,7 +455,19 @@ class Decoder
         return true;
     }
 
-    template<class T>
+    template <class T>
+    MOZ_WARN_UNUSED_RESULT bool
+    readEnum(T* out) {
+        static_assert(mozilla::IsEnum<T>::value, "is an enum");
+        // See Encoder::writeEnum.
+        uint8_t u8;
+        if (!read(&u8))
+            return false;
+        *out = T(u8);
+        return true;
+    }
+
+    template <class T>
     T uncheckedRead() {
         MOZ_ASSERT(uintptr_t(end_ - cur_) >= sizeof(T));
         T ret;
@@ -437,6 +476,13 @@ class Decoder
         return ret;
     }
 
+    template <class T>
+    T uncheckedReadEnum() {
+        // See Encoder::writeEnum.
+        static_assert(mozilla::IsEnum<T>::value, "is an enum");
+        return (T)uncheckedReadU8();
+    }
+
   public:
     explicit Decoder(const Bytecode& bytecode)
       : beg_(bytecode.begin()),
@@ -499,6 +545,9 @@ class Decoder
         *decoded |= uint32_t(byte) << 28;
         return true;
     }
+    MOZ_WARN_UNUSED_RESULT bool readExpr(Expr* expr) {
+        return readEnum(expr);
+    }
 
     // The infallible unpacking API should be used when we are sure that the
     // bytecode is well-formed.
@@ -539,6 +588,9 @@ class Decoder
         decoded |= uint32_t(byte) << 28;
         return decoded;
     }
+    Expr uncheckedReadExpr() {
+        return uncheckedReadEnum<Expr>();
+    }
 };
 
 // Source coordinates for a call site. As they're read sequentially, we

js/src/asmjs/WasmIonCompile.cpp

@@ -1180,8 +1180,7 @@ class FunctionCompiler
     double         readF64()    { return decoder_.uncheckedReadF64(); }
     SimdConstant   readI32X4()  { return decoder_.uncheckedReadI32X4(); }
     SimdConstant   readF32X4()  { return decoder_.uncheckedReadF32X4(); }
-
-    Expr           readOpcode() { return Expr(readU8()); }
+    Expr           readOpcode() { return decoder_.uncheckedReadExpr(); }
 
     void readCallLineCol(uint32_t* line, uint32_t* column) {
         const SourceCoords& sc = func_.sourceCoords(lastReadCallSite_++);