Rename isIntrinsicallyNondeterministic() to isGenerative() (#4092)

5 files changed, 31 insertions, 34 deletions

diff --git a/src/ir/properties.cpp b/src/ir/properties.cpp
index 889ece57b..fa4f299b3 100644
--- a/src/ir/properties.cpp
+++ b/src/ir/properties.cpp
@@ -21,20 +21,20 @@ namespace wasm {
 
 namespace Properties {
 
-bool isIntrinsicallyNondeterministic(Expression* curr, FeatureSet features) {
-  // Practically no wasm instructions are intrinsically nondeterministic.
-  // Exceptions occur only in GC atm.
+bool isGenerative(Expression* curr, FeatureSet features) {
+  // Practically no wasm instructions are generative. Exceptions occur only in
+  // GC atm.
   if (!features.hasGC()) {
     return false;
   }
 
   struct Scanner : public PostWalker<Scanner> {
-    bool deterministic = true;
-    void visitStructNew(StructNew* curr) { deterministic = false; }
-    void visitArrayNew(ArrayNew* curr) { deterministic = false; }
+    bool generative = false;
+    void visitStructNew(StructNew* curr) { generative = true; }
+    void visitArrayNew(ArrayNew* curr) { generative = true; }
   } scanner;
   scanner.walk(curr);
-  return !scanner.deterministic;
+  return scanner.generative;
 }
 
 } // namespace Properties
diff --git a/src/ir/properties.h b/src/ir/properties.h
index 1640ca814..f3cab1fdc 100644
--- a/src/ir/properties.h
+++ b/src/ir/properties.h
@@ -372,11 +372,10 @@ inline bool canEmitSelectWithArms(Expression* ifTrue, Expression* ifFalse) {
   return ifTrue->type.isSingle() && ifFalse->type.isSingle();
 }
 
-// An intrinsically-nondeterministic expression is one that can return different
-// results for the same inputs, and that difference is *not* explained by
-// other expressions that interact with this one. Hence the cause of
-// nondeterminism can be said to be "intrinsic" - it is internal and inherent in
-// the expression.
+// A "generative" expression is one that can generate different results for the
+// same inputs, and that difference is *not* explained by other expressions that
+// interact with this one. This is an intrinsic/internal property of the
+// expression.
 //
 // To see the issue more concretely, consider these:
 //
@@ -396,26 +395,26 @@ inline bool canEmitSelectWithArms(Expression* ifTrue, Expression* ifFalse) {
 // allocations, though, it doesn't matter what is in "..": there is nothing
 // in the wasm that we can check to find out if the results are the same or
 // not. (In fact, in this case they are always not the same.) So the
-// nondeterminism is "intrinsic."
+// generativity is "intrinsic" to the expression and it is because each call to
+// struct.new generates a new value.
 //
-// Thus, loads are nondeterministic but not intrinsically so, while GC
-// allocations are actual examples of intrinsically nondeterministic
-// instructions. If wasm were to add "get current time" or "get a random number"
-// instructions then those would also be intrinsically nondeterministic.
+// Thus, loads are nondeterministic but not generative, while GC allocations
+// are in fact generative. Note that "generative" need not mean "allocation" as
+// if wasm were to add "get current time" or "get a random number" instructions
+// then those would also be generative - generating a new current time value or
+// a new random number on each execution, respectively.
 //
-//  * Note that NaN nondeterminism is ignored here. Technically that allows e.g.
-//    an f32.add to be nondeterministic, but it is a valid wasm implementation
-//    to have deterministic NaN behavior, and we optimize under that assumption.
-//    So NaN nondeterminism does not cause anything to be intrinsically
-//    nondeterministic.
+//  * Note that NaN nondeterminism is ignored here. It is a valid wasm
+//    implementation to have deterministic NaN behavior, and we optimize under
+//    that simplifying assumption.
 //  * Note that calls are ignored here. In theory this concept could be defined
-//    either way for them (that is, we could potentially define them as either
-//    intrinsically nondeterministic, or not, and each could make sense in its
-//    own way). It is simpler to ignore them here, which means we only consider
-//    the behavior of the expression provided here (including its chldren), and
-//    not external code.
+//    either way for them - that is, we could potentially define them as
+//    generative, as they might contain such an instruction, or we could define
+//    this property as only looking at code in the current function. We choose
+//    the latter because calls are already handled best in other manners (using
+//    EffectAnalyzer).
 //
-bool isIntrinsicallyNondeterministic(Expression* curr, FeatureSet features);
+bool isGenerative(Expression* curr, FeatureSet features);
 
 } // namespace Properties
 
diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp
index 863179e49..d63c217de 100644
--- a/src/passes/LocalCSE.cpp
+++ b/src/passes/LocalCSE.cpp
@@ -436,8 +436,7 @@ struct Checker
       // nondeterministic: even if it has no side effects, if it may return a
       // different result each time, then we cannot optimize away repeats.
       if (effects.hasSideEffects() ||
-          Properties::isIntrinsicallyNondeterministic(curr,
-                                                      getModule()->features)) {
+          Properties::isGenerative(curr, getModule()->features)) {
         requestInfos.erase(curr);
       } else {
         activeOriginals.emplace(
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index cbf5d74d4..357f172d9 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -1583,8 +1583,7 @@ private:
     }
     // To be equal, they must also be known to return the same result
     // deterministically.
-    if (Properties::isIntrinsicallyNondeterministic(left,
-                                                    getModule()->features)) {
+    if (Properties::isGenerative(left, getModule()->features)) {
       return false;
     }
     return true;
diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp
index 89941a1ae..1ba298605 100644
--- a/src/passes/Precompute.cpp
+++ b/src/passes/Precompute.cpp
@@ -308,8 +308,8 @@ private:
         // creates a new, unique struct, even if the data is equal), and so
         // PrecomputingExpressionRunner will return a nonconstant flow for all
         // GC heap operations. (We could also have used
-        // Properties::isIntrinsicallyNondeterministic here, but that would be
-        // less efficient to re-scan the entire expression.)
+        // Properties::isGenerative here, but that would be less efficient to
+        // re-scan the entire expression.)
         //
         // (Other side effects are fine; if an expression does a call and we
         // somehow know the entire expression precomputes to a 42, then we can