Intrinsic: idempotent

scripts/test/fuzzing.py

@@ -116,6 +116,8 @@
     'vacuum-removable-if-unused.wast',
     'vacuum-removable-if-unused-func.wast',
     'strip-toolchain-annotations-func.wast',
+    'idempotent.wast',
+    'optimize-instructions_idempotent.wast',
     # Not fully implemented.
     'waitqueue.wast',
 ]

src/ir/intrinsics.h

@@ -116,7 +116,7 @@ class Intrinsics {
   std::vector<Name> getJSCalledFunctions();
 
   // Get the code annotations for an expression in a function.
-  CodeAnnotation getAnnotations(Expression* curr, Function* func) {
+  static CodeAnnotation getAnnotations(Expression* curr, Function* func) {
     auto& annotations = func->codeAnnotations;
     auto iter = annotations.find(curr);
     if (iter != annotations.end()) {
@@ -126,7 +126,7 @@ class Intrinsics {
   }
 
   // Get the code annotations for a function itself.
-  CodeAnnotation getAnnotations(Function* func) {
+  static CodeAnnotation getAnnotations(Function* func) {
     return getAnnotations(nullptr, func);
   }
 
@@ -155,6 +155,9 @@ class Intrinsics {
       if (!ret.jsCalled) {
         ret.jsCalled = funcAnnotations.jsCalled;
       }
+      if (!ret.idempotent) {
+        ret.idempotent = funcAnnotations.idempotent;
+      }
     }
 
     return ret;

src/ir/properties.cpp

@@ -15,6 +15,7 @@
  */
 
 #include "ir/properties.h"
+#include "ir/intrinsics.h"
 #include "wasm-traversal.h"
 
 namespace wasm::Properties {
@@ -25,7 +26,14 @@ struct GenerativityScanner : public PostWalker<GenerativityScanner> {
   bool generative = false;
 
   void visitCall(Call* curr) {
-    // TODO: We could in principle look at the called function to see if it is
+    // If the called function is idempotent, then it does not generate new
+    // values on each call.
+    if (Intrinsics(*getModule())
+          .getCallAnnotations(curr, getFunction())
+          .idempotent) {
+      return;
+    }
+    // TODO: We could look at the called function's contents to see if it is
     //       generative. To do that we'd need to compute generativity like we
     //       compute global effects (we can't just peek from here, as the
     //       other function might be modified in parallel).
@@ -43,15 +51,19 @@ struct GenerativityScanner : public PostWalker<GenerativityScanner> {
 
 } // anonymous namespace
 
-bool isGenerative(Expression* curr) {
+bool isGenerative(Expression* curr, Function* func, Module& wasm) {
   GenerativityScanner scanner;
+  scanner.setFunction(func);
+  scanner.setModule(&wasm);
   scanner.walk(curr);
   return scanner.generative;
 }
 
 // As above, but only checks |curr| and not children.
-bool isShallowlyGenerative(Expression* curr) {
+bool isShallowlyGenerative(Expression* curr, Function* func, Module& wasm) {
   GenerativityScanner scanner;
+  scanner.setFunction(func);
+  scanner.setModule(&wasm);
   scanner.visit(curr);
   return scanner.generative;
 }

src/ir/properties.h

@@ -597,10 +597,10 @@ inline bool hasUnwritableTypeImmediate(Expression* curr) {
 //    the latter because calls are already handled best in other manners (using
 //    EffectAnalyzer).
 //
-bool isGenerative(Expression* curr);
+bool isGenerative(Expression* curr, Function* func, Module& wasm);
 
 // As above, but only checks |curr| and not children.
-bool isShallowlyGenerative(Expression* curr);
+bool isShallowlyGenerative(Expression* curr, Function* func, Module& wasm);
 
 // Whether this expression is valid in a context where WebAssembly requires a
 // constant expression, such as a global initializer.

src/parser/contexts.h

@@ -1321,6 +1321,8 @@ struct AnnotationParserCtx {
         ret.removableIfUnused = true;
       } else if (a.kind == Annotations::JSCalledHint) {
         ret.jsCalled = true;
+      } else if (a.kind == Annotations::IdempotentHint) {
+        ret.idempotent = true;
       }
     }
 

src/passes/LocalCSE.cpp

@@ -398,7 +398,7 @@ struct Scanner
     // We also cannot optimize away something that is intrinsically
     // nondeterministic: even if it has no side effects, if it may return a
     // different result each time, and then we cannot optimize away repeats.
-    if (Properties::isShallowlyGenerative(curr)) {
+    if (Properties::isShallowlyGenerative(curr, getFunction(), *getModule())) {
       return false;
     }
 

src/passes/OptimizeInstructions.cpp

@@ -2822,35 +2822,17 @@ struct OptimizeInstructions
     }
 
     // To be equal, they must also be known to return the same result
-    // deterministically.
-    return !Properties::isGenerative(left);
-  }
-
-  // Similar to areConsecutiveInputsEqual() but also checks if we can remove
-  // them (but we do not assume the caller will always remove them).
-  bool areConsecutiveInputsEqualAndRemovable(Expression* left,
-                                             Expression* right) {
-    // First, check for side effects. If there are any, then we can't even
-    // assume things like local.get's of the same index being identical. (It is
-    // also ok to have removable side effects here, see the function
-    // description.)
-    auto& passOptions = getPassOptions();
-    if (EffectAnalyzer(passOptions, *getModule(), left)
-          .hasUnremovableSideEffects() ||
-        EffectAnalyzer(passOptions, *getModule(), right)
-          .hasUnremovableSideEffects()) {
-      return false;
-    }
-
-    return areConsecutiveInputsEqual(left, right);
+    // deterministically. We check the right side, as if the right is marked
+    // idempotent, that is enough (that tells us it does not generate a new
+    // value; logically, of course, as left is equal to right, they are calling
+    // the same thing, so it is odd to only annotate one, but this is consistent
+    // and easy to check).
+    return !Properties::isGenerative(right, getFunction(), *getModule());
   }
 
   // Check if two consecutive inputs to an instruction are equal and can also be
   // folded into the first of the two (but we do not assume the caller will
-  // always fold them). This is similar to areConsecutiveInputsEqualAndRemovable
-  // but also identifies reads from the same local variable when the first of
-  // them is a "tee" operation and the second is a get (in which case, it is
-  // fine to remove the get, but not the tee).
+  // always fold them).
   //
   // The inputs here must be consecutive, but it is also ok to have code with no
   // side effects at all in the middle. For example, a Const in between is ok.
@@ -2862,9 +2844,55 @@ struct OptimizeInstructions
       return true;
     }
 
-    // stronger property than we need - we can not only fold
-    // them but remove them entirely.
-    return areConsecutiveInputsEqualAndRemovable(left, right);
+    // To fold the right side into the left, it must have no effects.
+    auto rightMightHaveEffects = true;
+    if (auto* call = right->dynCast<Call>()) {
+      // If these are a pair of idempotent calls, then the second has no
+      // effects. (We didn't check if left is a call, but the equality check
+      // below does that.)
+      if (Intrinsics(*getModule())
+            .getCallAnnotations(call, getFunction())
+            .idempotent) {
+        // We must still check for effects in the parameters. Imagine that we
+        // have
+        //
+        //  (call $idempotent (global.get $g))
+        //  (call $idempotent (global.get $g))
+        //
+        // Then the first call has effects, and those might alter $g if the
+        // global is mutable. That is, all that idempotency tells us is that
+        // the second call has no effects, but its parameters can still have
+        // read effects that interact. Also, the parameter might have write
+        // effects,
+        //
+        //  (call $idempotent (call $other))
+        //
+        // We must check that as well.
+        EffectAnalyzer childEffects(getPassOptions(), *getModule());
+        for (auto* child : call->operands) {
+          childEffects.walk(child);
+        }
+        if (childEffects.hasUnremovableSideEffects()) {
+          return false;
+        }
+        ShallowEffectAnalyzer parentEffects(
+          getPassOptions(), *getModule(), call);
+        if (parentEffects.invalidates(childEffects)) {
+          return false;
+        }
+        // No effects are possible.
+        rightMightHaveEffects = false;
+      }
+    }
+    if (rightMightHaveEffects) {
+      // So far it looks like right has effects, so check fully.
+      if (EffectAnalyzer(getPassOptions(), *getModule(), right)
+            .hasUnremovableSideEffects()) {
+        return false;
+      }
+    }
+
+    return areConsecutiveInputsEqual(left, right);
   }
 
   // Canonicalizing the order of a symmetric binary helps us

src/passes/Print.cpp

@@ -2803,6 +2803,12 @@ void PrintSExpression::printCodeAnnotations(Expression* curr) {
       restoreNormalColor(o);
       doIndent(o, indent);
     }
+    if (annotation.idempotent) {
+      Colors::grey(o);
+      o << "(@" << Annotations::IdempotentHint << ")\n";
+      restoreNormalColor(o);
+      doIndent(o, indent);
+    }
   }
 }
 

src/passes/StripToolchainAnnotations.cpp

@@ -44,6 +44,7 @@ struct StripToolchainAnnotations
       auto& annotation = iter->second;
       annotation.removableIfUnused = false;
       annotation.jsCalled = false;
+      annotation.idempotent = false;
 
       // If nothing remains, remove the entire annotation.
       if (annotation == CodeAnnotation()) {

src/wasm-annotations.h

@@ -29,6 +29,7 @@ extern const Name BranchHint;
 extern const Name InlineHint;
 extern const Name RemovableIfUnusedHint;
 extern const Name JSCalledHint;
+extern const Name IdempotentHint;
 
 } // namespace wasm::Annotations
 

src/wasm-binary.h

@@ -1444,6 +1444,7 @@ class WasmBinaryWriter {
   std::optional<BufferWithRandomAccess> getInlineHintsBuffer();
   std::optional<BufferWithRandomAccess> getRemovableIfUnusedHintsBuffer();
   std::optional<BufferWithRandomAccess> getJSCalledHintsBuffer();
+  std::optional<BufferWithRandomAccess> getIdempotentHintsBuffer();
 
   // helpers
   void writeInlineString(std::string_view name);
@@ -1738,6 +1739,7 @@ class WasmBinaryReader {
   void readInlineHints(size_t payloadLen);
   void readRemovableIfUnusedHints(size_t payloadLen);
   void readJSCalledHints(size_t payloadLen);
+  void readIdempotentHints(size_t payloadLen);
 
   std::tuple<Address, Address, Index, MemoryOrder>
   readMemoryAccess(bool isAtomic, bool isRMW);

src/wasm.h

@@ -2258,10 +2258,16 @@ struct CodeAnnotation {
   // identity does not matter for such functions.
   bool jsCalled = false;
 
+  // A function that may do something on the first call, but all subsequent
+  // calls with the same parameters can be assumed to have no effects. If a
+  // value is returned, it will be the same value as returned earlier (for the
+  // same parameters).
+  bool idempotent = false;
+
   bool operator==(const CodeAnnotation& other) const {
     return branchLikely == other.branchLikely && inline_ == other.inline_ &&
            removableIfUnused == other.removableIfUnused &&
-           jsCalled == other.jsCalled;
+           jsCalled == other.jsCalled && idempotent == other.idempotent;
   }
 };
 
@@ -2322,6 +2328,13 @@ class Function : public Importable {
   // the 0 byte offset in the spec. As with debug info, we do not store these on
   // Expressions as we assume most instances are unannotated, and do not want to
   // add constant memory overhead.
+  // XXX As an unordered map, if this is modified by one thread, another should
+  //     not be reading it. That should not happen atm - all annotations are
+  //     set up in dedicated passes or in the binary reader - but if one pass
+  //     could add an expression annotation, another should not at the same time
+  //     read the function-level annotations, even though that is natural to do.
+  //     We may want to move the function-level annotations to a dedicated
+  //     field outside the map.
   std::unordered_map<Expression*, CodeAnnotation> codeAnnotations;
 
   // The effects for this function, if they have been computed. We use a shared