Intrinsic: idempotent (#8354)

An idempotent function may do work when called, but if called again with
the same parameters, it does no work and it returns the same value (if
it returns a value).

This uses the new intrinsic in OptimizeInstructions. More things are
possible too.

See discussion in #7574

Author: kripken

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);
   }
 
@@ -165,6 +165,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,8 +2822,12 @@ struct OptimizeInstructions
     }
 
     // To be equal, they must also be known to return the same result
-    // deterministically.
-    return !Properties::isGenerative(left);
+    // 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
@@ -2841,9 +2845,51 @@ struct OptimizeInstructions
     }
 
     // To fold the right side into the left, it must have no effects.
-    if (EffectAnalyzer(getPassOptions(), *getModule(), right)
-          .hasUnremovableSideEffects()) {
-      return false;
+    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);

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