Manual changes

Author: stevenfontanella

src/interpreter/interpreter.cpp

@@ -216,8 +216,8 @@ struct ExpressionInterpreter : OverriddenVisitor<ExpressionInterpreter, Flow> {
         WASM_UNREACHABLE("TODO");
     }
   }
-  Flow visitSelect(Select* curr) { WASM_UNREACHABLE("TODO"); }
   Flow visitWideIntAddSub(WideIntAddSub* curr) { WASM_UNREACHABLE("TODO"); }
+  Flow visitSelect(Select* curr) { WASM_UNREACHABLE("TODO"); }
   Flow visitDrop(Drop* curr) { WASM_UNREACHABLE("TODO"); }
   Flow visitReturn(Return* curr) { WASM_UNREACHABLE("TODO"); }
   Flow visitMemorySize(MemorySize* curr) { WASM_UNREACHABLE("TODO"); }

src/passes/Precompute.cpp

@@ -282,10 +282,6 @@ class PrecomputingExpressionRunner
     return Flow(NONCONSTANT_FLOW);
   }
 
-  Flow visitWideIntAddSub(WideIntAddSub* curr) {
-    return Super::visitWideIntAddSub(curr);
-  }
-
   Flow visitStringEncode(StringEncode* curr) {
     // string.encode_wtf16_array is effectively an Array write operation, so
     // just like ArraySet and ArrayCopy above we must mark it as disallowed

src/support/stdckdint.h

@@ -38,6 +38,22 @@ template<typename T> bool ckd_add(T* output, T a, T b) {
 #endif
 }
 
+template<typename T> bool ckd_sub(T* output, T a, T b) {
+#if __has_builtin(__builtin_sub_overflow)
+  return __builtin_sub_overflow(a, b, output);
+#else
+  // Atm this polyfill only supports unsigned types.
+  static_assert(std::is_unsigned_v<T>);
+
+  T result = a - b;
+  if (result > a) {
+    return true;
+  }
+  *output = result;
+  return false;
+#endif
+}
+
 } // namespace std
 
 #endif // wasm_stdckdint_h

src/wasm-interpreter.h

@@ -1808,18 +1808,25 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
       uint64_t lowRHS = rightLow.getSingleValue().geti64();
       uint64_t highRHS = rightHigh.getSingleValue().geti64();
 
-      uint64_t lowRes, highRes;
-      if (curr->op == AddInt128) {
-        lowRes = lowLHS + lowRHS;
-        highRes = highLHS + highRHS + (lowRes < lowLHS);
-      } else {
-        lowRes = lowLHS - lowRHS;
-        highRes = highLHS - highRHS - (lowLHS < lowRHS);
+      uint64_t lowResult = 0;
+      uint64_t highResult = 0;
+
+      switch (curr->op) {
+        case AddInt128: {
+          bool overflowed = std::ckd_add(&lowResult, lowLHS, lowRHS);
+          highResult = highLHS + highRHS + overflowed;
+          break;
+        }
+        case SubInt128: {
+          bool overflowed = std::ckd_sub(&lowResult, lowLHS, lowRHS);
+          highResult = highLHS - highRHS - overflowed;
+          break;
+        }
       }
 
       Literals results;
-      results.push_back(Literal(lowRes));
-      results.push_back(Literal(highRes));
+      results.push_back(Literal(lowResult));
+      results.push_back(Literal(highResult));
       return results;
     }
     WASM_UNREACHABLE("invalid wide int binary op");
@@ -3074,9 +3081,6 @@ class ConstantExpressionRunner : public ExpressionRunner<SubType> {
   Flow visitAtomicCmpxchg(AtomicCmpxchg* curr) {
     return Flow(NONCONSTANT_FLOW);
   }
-  Flow visitWideIntAddSub(WideIntAddSub* curr) {
-    return ExpressionRunner<SubType>::visitWideIntAddSub(curr);
-  }
   Flow visitAtomicWait(AtomicWait* curr) { return Flow(NONCONSTANT_FLOW); }
   Flow visitAtomicNotify(AtomicNotify* curr) { return Flow(NONCONSTANT_FLOW); }
   Flow visitStructWait(StructWait* curr) { return Flow(NONCONSTANT_FLOW); }

src/wasm/wasm-stack.cpp

@@ -2290,14 +2290,14 @@ void BinaryInstWriter::visitSelect(Select* curr) {
 void BinaryInstWriter::visitWideIntAddSub(WideIntAddSub* curr) {
   o << static_cast<int8_t>(BinaryConsts::MiscPrefix);
   switch (curr->op) {
-    case AddInt128:
+    case AddInt128: {
       o << U32LEB(BinaryConsts::I64Add128);
       break;
-    case SubInt128:
+    }
+    case SubInt128: {
       o << U32LEB(BinaryConsts::I64Sub128);
       break;
-    default:
-      WASM_UNREACHABLE("invalid wide int binary op");
+    }
   }
 }
 

src/wasm/wasm-validator.cpp

@@ -173,9 +173,9 @@ struct ValidationInfo {
     return true;
   }
 
-  template<typename T, typename S>
+  template<typename T>
   bool shouldBeEqualOrFirstIsUnreachable(
-    S left, S right, T curr, const char* text, Function* func = nullptr) {
+    Type left, Type right, T curr, const char* text, Function* func = nullptr) {
     if (left != Type::unreachable && left != right) {
       std::ostringstream ss;
       ss << left << " != " << right << ": " << text;
@@ -608,9 +608,9 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
     return info.shouldBeEqual(left, right, curr, text, getFunction());
   }
 
-  template<typename T, typename S>
+  template<typename T>
   bool
-  shouldBeEqualOrFirstIsUnreachable(S left, S right, T curr, const char* text) {
+  shouldBeEqualOrFirstIsUnreachable(Type left, Type right, T curr, const char* text) {
     return info.shouldBeEqualOrFirstIsUnreachable(
       left, right, curr, text, getFunction());
   }
@@ -2443,19 +2443,16 @@ void FunctionValidator::visitSelect(Select* curr) {
 }
 
 void FunctionValidator::visitWideIntAddSub(WideIntAddSub* curr) {
-  if (!shouldBeTrue(getModule()->features.hasWideArithmetic(),
+  shouldBeTrue(getModule()->features.hasWideArithmetic(),
                     curr,
-                    "Wide arithmetic is not enabled")) {
-    return;
-  }
+                    "i64.add128 / i64.sub128 require wide arithmetic [--enable-wide-arithmetic]");
+
   for (auto* operand :
        {curr->leftLow, curr->leftHigh, curr->rightLow, curr->rightHigh}) {
-    if (operand->type != Type::unreachable) {
-      shouldBeEqual(operand->type,
-                    Type(Type::i64),
-                    curr,
-                    "wide binary child types must be i64");
-    }
+    shouldBeEqualOrFirstIsUnreachable(operand->type,
+                  Type(Type::BasicType::i64),
+                  curr,
+                  "wide binary child types must be i64");
   }
 }
 

test/spec/wide-arithmetic.wast

@@ -1,4 +1,3 @@
-;;; --enable-wide-arithmetic
 (module
   (func (export "i64.add128") (param i64 i64 i64 i64) (result i64 i64)
     local.get 0