Add a Module parameter to EffectAnalyzer. NFC (#4115)

Knowing the module will allow us to do more analysis in the
effect analyzer. For now, this just refactors the code to allow
providing a module instead of features, and to infer the
features from the module. This actually shortens the code in
most places which is nice (just pass module instead of
module->features).

This modifies basically all callers to use the new module
form, except for the fallthrough logic. That would require
some more refactoring, so to keep this PR reasonably small
that is not yet done.

14 files changed, 115 insertions, 110 deletions

diff --git a/src/ir/block-utils.h b/src/ir/block-utils.h
index f8224186f..be24cf483 100644
--- a/src/ir/block-utils.h
+++ b/src/ir/block-utils.h
@@ -37,10 +37,9 @@ simplifyToContents(Block* block, T* parent, bool allowTypeChange = false) {
       !BranchUtils::BranchSeeker::has(list[0], block->name)) {
     // just one element. try to replace the block
     auto* singleton = list[0];
-    auto sideEffects = EffectAnalyzer(parent->getPassOptions(),
-                                      parent->getModule()->features,
-                                      singleton)
-                         .hasSideEffects();
+    auto sideEffects =
+      EffectAnalyzer(parent->getPassOptions(), *parent->getModule(), singleton)
+        .hasSideEffects();
     if (!sideEffects && !singleton->type.isConcrete()) {
       // no side effects, and singleton is not returning a value, so we can
       // throw away the block and its contents, basically
diff --git a/src/ir/effects.h b/src/ir/effects.h
index e8fba8e3a..d34f8e5b2 100644
--- a/src/ir/effects.h
+++ b/src/ir/effects.h
@@ -31,7 +31,19 @@ public:
                  Expression* ast = nullptr)
     : ignoreImplicitTraps(passOptions.ignoreImplicitTraps),
       trapsNeverHappen(passOptions.trapsNeverHappen),
-      debugInfo(passOptions.debugInfo), features(features) {
+      debugInfo(passOptions.debugInfo), module(nullptr), features(features) {
+    if (ast) {
+      walk(ast);
+    }
+  }
+
+  EffectAnalyzer(const PassOptions& passOptions,
+                 Module& module,
+                 Expression* ast = nullptr)
+    : ignoreImplicitTraps(passOptions.ignoreImplicitTraps),
+      trapsNeverHappen(passOptions.trapsNeverHappen),
+      debugInfo(passOptions.debugInfo), module(&module),
+      features(module.features) {
     if (ast) {
       walk(ast);
     }
@@ -40,6 +52,7 @@ public:
   bool ignoreImplicitTraps;
   bool trapsNeverHappen;
   bool debugInfo;
+  Module* module;
   FeatureSet features;
 
   // Walk an expression and all its children.
@@ -680,6 +693,15 @@ public:
     return !aEffects.invalidates(bEffects);
   }
 
+  static bool canReorder(const PassOptions& passOptions,
+                         Module& module,
+                         Expression* a,
+                         Expression* b) {
+    EffectAnalyzer aEffects(passOptions, module, a);
+    EffectAnalyzer bEffects(passOptions, module, b);
+    return !aEffects.invalidates(bEffects);
+  }
+
   // C-API
 
   enum SideEffects : uint32_t {
@@ -767,9 +789,9 @@ private:
 class ShallowEffectAnalyzer : public EffectAnalyzer {
 public:
   ShallowEffectAnalyzer(const PassOptions& passOptions,
-                        FeatureSet features,
+                        Module& module,
                         Expression* ast = nullptr)
-    : EffectAnalyzer(passOptions, features) {
+    : EffectAnalyzer(passOptions, module) {
     if (ast) {
       visit(ast);
     }
diff --git a/src/ir/properties.h b/src/ir/properties.h
index dbf2715ef..2e3f94a06 100644
--- a/src/ir/properties.h
+++ b/src/ir/properties.h
@@ -250,6 +250,8 @@ inline Index getZeroExtBits(Expression* curr) {
 // child of this expression. See getFallthrough for a method that looks all the
 // way to the final value falling through, potentially through multiple
 // intermediate expressions.
+//
+// TODO: Receive a Module instead of FeatureSet, to pass to EffectAnalyzer?
 inline Expression* getImmediateFallthrough(Expression* curr,
                                            const PassOptions& passOptions,
                                            FeatureSet features) {
diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp
index 5b7c88024..886b5b891 100644
--- a/src/passes/CodeFolding.cpp
+++ b/src/passes/CodeFolding.cpp
@@ -303,7 +303,7 @@ private:
         return false;
       }
       if (getModule()->features.hasExceptionHandling()) {
-        EffectAnalyzer effects(getPassOptions(), getModule()->features, item);
+        EffectAnalyzer effects(getPassOptions(), *getModule(), item);
         // Pop instructions are pseudoinstructions used only after 'catch' to
         // simulate its behavior. We cannot move expressions containing pops if
         // they are not enclosed in a 'catch' body, because a pop instruction
@@ -591,9 +591,8 @@ private:
                                 // TODO: this should not be a problem in
                                 //       *non*-terminating tails, but
                                 //       double-verify that
-                                if (EffectAnalyzer(getPassOptions(),
-                                                   getModule()->features,
-                                                   newItem)
+                                if (EffectAnalyzer(
+                                      getPassOptions(), *getModule(), newItem)
                                       .hasExternalBreakTargets()) {
                                   return true;
                                 }
diff --git a/src/passes/DeadArgumentElimination.cpp b/src/passes/DeadArgumentElimination.cpp
index 419ae1aca..afe9f19b7 100644
--- a/src/passes/DeadArgumentElimination.cpp
+++ b/src/passes/DeadArgumentElimination.cpp
@@ -395,7 +395,7 @@ struct DAE : public Pass {
           bool callParamsAreValid =
             std::none_of(calls.begin(), calls.end(), [&](Call* call) {
               auto* operand = call->operands[i];
-              return EffectAnalyzer(runner->options, module->features, operand)
+              return EffectAnalyzer(runner->options, *module, operand)
                 .hasSideEffects();
             });
           // The type must be valid for us to handle as a local (since we
diff --git a/src/passes/LocalCSE.cpp b/src/passes/LocalCSE.cpp
index a233517c5..863179e49 100644
--- a/src/passes/LocalCSE.cpp
+++ b/src/passes/LocalCSE.cpp
@@ -380,7 +380,7 @@ struct Checker
     // Given the current expression, see what it invalidates of the currently-
     // hashed expressions, if there are any.
     if (!activeOriginals.empty()) {
-      EffectAnalyzer effects(options, getModule()->features);
+      EffectAnalyzer effects(options, *getModule());
       // We only need to visit this node itself, as we have already visited its
       // children by the time we get here.
       effects.visit(curr);
@@ -421,7 +421,7 @@ struct Checker
     if (info.requests > 0) {
       // This is an original. Compute its side effects, as we cannot optimize
       // away repeated apperances if it has any.
-      EffectAnalyzer effects(options, getModule()->features, curr);
+      EffectAnalyzer effects(options, *getModule(), curr);
 
       // We can ignore traps here, as we replace a repeating expression with a
       // single appearance of it, a store to a local, and gets in the other
diff --git a/src/passes/LoopInvariantCodeMotion.cpp b/src/passes/LoopInvariantCodeMotion.cpp
index a80e3a55d..1007e57b3 100644
--- a/src/passes/LoopInvariantCodeMotion.cpp
+++ b/src/passes/LoopInvariantCodeMotion.cpp
@@ -60,14 +60,13 @@ struct LoopInvariantCodeMotion
     // Accumulate effects of things we can't move out - things
     // we move out later must cross them, so we must verify it
     // is ok to do so.
-    FeatureSet features = getModule()->features;
-    EffectAnalyzer effectsSoFar(getPassOptions(), features);
+    EffectAnalyzer effectsSoFar(getPassOptions(), *getModule());
     // The loop's total effects also matter. For example, a store
     // in the loop means we can't move a load outside.
     // FIXME: we look at the loop "tail" area too, after the last
     //        possible branch back, which can cause false positives
     //        for bad effect interactions.
-    EffectAnalyzer loopEffects(getPassOptions(), features, loop);
+    EffectAnalyzer loopEffects(getPassOptions(), *getModule(), loop);
     // Note all the sets in each loop, and how many per index. Currently
     // EffectAnalyzer can't do that, and we need it to know if we
     // can move a set out of the loop (if there is another set
@@ -108,7 +107,7 @@ struct LoopInvariantCodeMotion
         // a branch to it anyhow, so we would stop before that point anyhow.
       }
       // If this may branch, we are done.
-      EffectAnalyzer effects(getPassOptions(), features, curr);
+      EffectAnalyzer effects(getPassOptions(), *getModule(), curr);
       if (effects.transfersControlFlow()) {
         break;
       }
diff --git a/src/passes/MergeBlocks.cpp b/src/passes/MergeBlocks.cpp
index 7e8819309..b38f949e9 100644
--- a/src/passes/MergeBlocks.cpp
+++ b/src/passes/MergeBlocks.cpp
@@ -113,7 +113,7 @@ struct ProblemFinder
           brIfs++;
         }
         // if the value has side effects, we can't remove it
-        if (EffectAnalyzer(passOptions, getModule()->features, br->value)
+        if (EffectAnalyzer(passOptions, *getModule(), br->value)
               .hasSideEffects()) {
           foundProblem = true;
         }
@@ -434,18 +434,17 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
     if (!child) {
       return outer;
     }
-    FeatureSet features = getModule()->features;
     if ((dependency1 && *dependency1) || (dependency2 && *dependency2)) {
       // there are dependencies, things we must be reordered through. make sure
       // no problems there
-      EffectAnalyzer childEffects(getPassOptions(), features, child);
+      EffectAnalyzer childEffects(getPassOptions(), *getModule(), child);
       if (dependency1 && *dependency1 &&
-          EffectAnalyzer(getPassOptions(), features, *dependency1)
+          EffectAnalyzer(getPassOptions(), *getModule(), *dependency1)
             .invalidates(childEffects)) {
         return outer;
       }
       if (dependency2 && *dependency2 &&
-          EffectAnalyzer(getPassOptions(), features, *dependency2)
+          EffectAnalyzer(getPassOptions(), *getModule(), *dependency2)
             .invalidates(childEffects)) {
         return outer;
       }
@@ -512,17 +511,19 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
                        Expression*& third) {
     // TODO: for now, just stop when we see any side effect. instead, we could
     //       check effects carefully for reordering
-    FeatureSet features = getModule()->features;
     Block* outer = nullptr;
-    if (EffectAnalyzer(getPassOptions(), features, first).hasSideEffects()) {
+    if (EffectAnalyzer(getPassOptions(), *getModule(), first)
+          .hasSideEffects()) {
       return;
     }
     outer = optimize(curr, first, outer);
-    if (EffectAnalyzer(getPassOptions(), features, second).hasSideEffects()) {
+    if (EffectAnalyzer(getPassOptions(), *getModule(), second)
+          .hasSideEffects()) {
       return;
     }
     outer = optimize(curr, second, outer);
-    if (EffectAnalyzer(getPassOptions(), features, third).hasSideEffects()) {
+    if (EffectAnalyzer(getPassOptions(), *getModule(), third)
+          .hasSideEffects()) {
       return;
     }
     optimize(curr, third, outer);
@@ -548,8 +549,7 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
   template<typename T> void handleCall(T* curr) {
     Block* outer = nullptr;
     for (Index i = 0; i < curr->operands.size(); i++) {
-      if (EffectAnalyzer(
-            getPassOptions(), getModule()->features, curr->operands[i])
+      if (EffectAnalyzer(getPassOptions(), *getModule(), curr->operands[i])
             .hasSideEffects()) {
         return;
       }
@@ -560,16 +560,15 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
   void visitCall(Call* curr) { handleCall(curr); }
 
   template<typename T> void handleNonDirectCall(T* curr) {
-    FeatureSet features = getModule()->features;
     Block* outer = nullptr;
     for (Index i = 0; i < curr->operands.size(); i++) {
-      if (EffectAnalyzer(getPassOptions(), features, curr->operands[i])
+      if (EffectAnalyzer(getPassOptions(), *getModule(), curr->operands[i])
             .hasSideEffects()) {
         return;
       }
       outer = optimize(curr, curr->operands[i], outer);
     }
-    if (EffectAnalyzer(getPassOptions(), features, curr->target)
+    if (EffectAnalyzer(getPassOptions(), *getModule(), curr->target)
           .hasSideEffects()) {
       return;
     }
@@ -583,8 +582,7 @@ struct MergeBlocks : public WalkerPass<PostWalker<MergeBlocks>> {
   void visitThrow(Throw* curr) {
     Block* outer = nullptr;
     for (Index i = 0; i < curr->operands.size(); i++) {
-      if (EffectAnalyzer(
-            getPassOptions(), getModule()->features, curr->operands[i])
+      if (EffectAnalyzer(getPassOptions(), *getModule(), curr->operands[i])
             .hasSideEffects()) {
         return;
       }
diff --git a/src/passes/OptimizeInstructions.cpp b/src/passes/OptimizeInstructions.cpp
index 2c4bdc905..22d53b151 100644
--- a/src/passes/OptimizeInstructions.cpp
+++ b/src/passes/OptimizeInstructions.cpp
@@ -275,7 +275,7 @@ struct OptimizeInstructions
   }
 
   EffectAnalyzer effects(Expression* expr) {
-    return EffectAnalyzer(getPassOptions(), getModule()->features, expr);
+    return EffectAnalyzer(getPassOptions(), *getModule(), expr);
   }
 
   decltype(auto) pure(Expression** binder) {
@@ -284,8 +284,7 @@ struct OptimizeInstructions
   }
 
   bool canReorder(Expression* a, Expression* b) {
-    return EffectAnalyzer::canReorder(
-      getPassOptions(), getModule()->features, a, b);
+    return EffectAnalyzer::canReorder(getPassOptions(), *getModule(), a, b);
   }
 
   void visitBinary(Binary* curr) {
@@ -1379,7 +1378,8 @@ struct OptimizeInstructions
     if (auto* child = ref->dynCast<RefCast>()) {
       // Check if the casts are identical.
       if (ExpressionAnalyzer::equal(curr->rtt, child->rtt) &&
-          !EffectAnalyzer(passOptions, features, curr->rtt).hasSideEffects()) {
+          !EffectAnalyzer(passOptions, *getModule(), curr->rtt)
+             .hasSideEffects()) {
         replaceCurrent(curr->ref);
         return;
       }
@@ -1545,9 +1545,9 @@ private:
     // checking if we can remove the two inputs anyhow.)
     auto passOptions = getPassOptions();
     auto features = getModule()->features;
-    if (EffectAnalyzer(passOptions, features, left)
+    if (EffectAnalyzer(passOptions, *getModule(), left)
           .hasUnremovableSideEffects() ||
-        EffectAnalyzer(passOptions, features, right)
+        EffectAnalyzer(passOptions, *getModule(), right)
           .hasUnremovableSideEffects()) {
       return false;
     }
@@ -1916,7 +1916,6 @@ private:
         if (!curr->type.isInteger()) {
           return;
         }
-        FeatureSet features = getModule()->features;
         auto type = curr->type;
         auto* left = curr->left->dynCast<Const>();
         auto* right = curr->right->dynCast<Const>();
@@ -1938,7 +1937,7 @@ private:
           // shift has side effects
           if (((left && left->value.isZero()) ||
                (right && Bits::getEffectiveShifts(right) == 0)) &&
-              !EffectAnalyzer(passOptions, features, curr->right)
+              !EffectAnalyzer(passOptions, *getModule(), curr->right)
                  .hasSideEffects()) {
             replaceCurrent(curr->left);
             return;
@@ -1947,13 +1946,13 @@ private:
           // multiplying by zero is a zero, unless the other side has side
           // effects
           if (left && left->value.isZero() &&
-              !EffectAnalyzer(passOptions, features, curr->right)
+              !EffectAnalyzer(passOptions, *getModule(), curr->right)
                  .hasSideEffects()) {
             replaceCurrent(left);
             return;
           }
           if (right && right->value.isZero() &&
-              !EffectAnalyzer(passOptions, features, curr->left)
+              !EffectAnalyzer(passOptions, *getModule(), curr->left)
                  .hasSideEffects()) {
             replaceCurrent(right);
             return;
@@ -2687,8 +2686,7 @@ private:
     if (type.isInteger()) {
       if (auto* inner = outer->right->dynCast<Binary>()) {
         if (outer->op == inner->op) {
-          if (!EffectAnalyzer(
-                 getPassOptions(), getModule()->features, outer->left)
+          if (!EffectAnalyzer(getPassOptions(), *getModule(), outer->left)
                  .hasSideEffects()) {
             if (ExpressionAnalyzer::equal(inner->left, outer->left)) {
               // x - (x - y)  ==>   y
@@ -2730,8 +2728,7 @@ private:
       }
       if (auto* inner = outer->left->dynCast<Binary>()) {
         if (outer->op == inner->op) {
-          if (!EffectAnalyzer(
-                 getPassOptions(), getModule()->features, outer->right)
+          if (!EffectAnalyzer(getPassOptions(), *getModule(), outer->right)
                  .hasSideEffects()) {
             if (ExpressionAnalyzer::equal(inner->right, outer->right)) {
               // (x ^ y) ^ y  ==>   x
@@ -3263,9 +3260,8 @@ private:
             // the side effects execute once, so there is no problem.
             // TODO: handle certain side effects when possible in select
             bool validEffects = std::is_same<T, If>::value ||
-                                !ShallowEffectAnalyzer(getPassOptions(),
-                                                       getModule()->features,
-                                                       curr->ifTrue)
+                                !ShallowEffectAnalyzer(
+                                   getPassOptions(), *getModule(), curr->ifTrue)
                                    .hasSideEffects();
 
             // In addition, check for specific limitations of select.
diff --git a/src/passes/RemoveUnusedBrs.cpp b/src/passes/RemoveUnusedBrs.cpp
index f80f9ba39..ea8a66612 100644
--- a/src/passes/RemoveUnusedBrs.cpp
+++ b/src/passes/RemoveUnusedBrs.cpp
@@ -65,7 +65,7 @@ stealSlice(Builder& builder, Block* input, Index from, Index to) {
 static bool canTurnIfIntoBrIf(Expression* ifCondition,
                               Expression* brValue,
                               PassOptions& options,
-                              FeatureSet features) {
+                              Module& wasm) {
   // if the if isn't even reached, this is all dead code anyhow
   if (ifCondition->type == Type::unreachable) {
     return false;
@@ -73,11 +73,11 @@ static bool canTurnIfIntoBrIf(Expression* ifCondition,
   if (!brValue) {
     return true;
   }
-  EffectAnalyzer value(options, features, brValue);
+  EffectAnalyzer value(options, wasm, brValue);
   if (value.hasSideEffects()) {
     return false;
   }
-  return !EffectAnalyzer(options, features, ifCondition).invalidates(value);
+  return !EffectAnalyzer(options, wasm, ifCondition).invalidates(value);
 }
 
 // Check if it is not worth it to run code unconditionally. This
@@ -329,13 +329,12 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
   }
 
   void visitIf(If* curr) {
-    FeatureSet features = getModule()->features;
     if (!curr->ifFalse) {
       // if without an else. try to reduce
       //    if (condition) br  =>  br_if (condition)
       if (Break* br = curr->ifTrue->dynCast<Break>()) {
         if (canTurnIfIntoBrIf(
-              curr->condition, br->value, getPassOptions(), features)) {
+              curr->condition, br->value, getPassOptions(), *getModule())) {
           if (!br->condition) {
             br->condition = curr->condition;
           } else {
@@ -360,7 +359,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
             }
             // Of course we can't do this if the br's condition has side
             // effects, as we would then execute those unconditionally.
-            if (EffectAnalyzer(getPassOptions(), features, br->condition)
+            if (EffectAnalyzer(getPassOptions(), *getModule(), br->condition)
                   .hasSideEffects()) {
               return;
             }
@@ -555,7 +554,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
         return false;
       }
       // if there is control flow, we must stop looking
-      if (EffectAnalyzer(getPassOptions(), getModule()->features, curr)
+      if (EffectAnalyzer(getPassOptions(), *getModule(), curr)
             .transfersControlFlow()) {
         return false;
       }
@@ -851,7 +850,6 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
         //   the if is dead
         // * note that we do this at the end, because un-conditionalizing can
         //   interfere with optimizeLoop()ing.
-        FeatureSet features = getModule()->features;
         auto& list = curr->list;
         for (Index i = 0; i < list.size(); i++) {
           auto* iff = list[i]->dynCast<If>();
@@ -862,8 +860,10 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           }
           auto* ifTrueBreak = iff->ifTrue->dynCast<Break>();
           if (ifTrueBreak && !ifTrueBreak->condition &&
-              canTurnIfIntoBrIf(
-                iff->condition, ifTrueBreak->value, passOptions, features)) {
+              canTurnIfIntoBrIf(iff->condition,
+                                ifTrueBreak->value,
+                                passOptions,
+                                *getModule())) {
             // we are an if-else where the ifTrue is a break without a
             // condition, so we can do this
             ifTrueBreak->condition = iff->condition;
@@ -875,8 +875,10 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           // otherwise, perhaps we can flip the if
           auto* ifFalseBreak = iff->ifFalse->dynCast<Break>();
           if (ifFalseBreak && !ifFalseBreak->condition &&
-              canTurnIfIntoBrIf(
-                iff->condition, ifFalseBreak->value, passOptions, features)) {
+              canTurnIfIntoBrIf(iff->condition,
+                                ifFalseBreak->value,
+                                passOptions,
+                                *getModule())) {
             ifFalseBreak->condition =
               Builder(*getModule()).makeUnary(EqZInt32, iff->condition);
             ifFalseBreak->finalize();
@@ -905,7 +907,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
               if (shrink && br2->type != Type::unreachable) {
                 // Join adjacent br_ifs to the same target, making one br_if
                 // with a "selectified" condition that executes both.
-                if (!EffectAnalyzer(passOptions, features, br2->condition)
+                if (!EffectAnalyzer(passOptions, *getModule(), br2->condition)
                        .hasSideEffects()) {
                   // it's ok to execute them both, do it
                   Builder builder(*getModule());
@@ -933,10 +935,9 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           // This switch has just one target no matter what; replace with a br
           // if we can (to do so, we must put the condition before a possible
           // value).
-          if (!curr->value || EffectAnalyzer::canReorder(passOptions,
-                                                         getModule()->features,
-                                                         curr->condition,
-                                                         curr->value)) {
+          if (!curr->value ||
+              EffectAnalyzer::canReorder(
+                passOptions, *getModule(), curr->condition, curr->value)) {
             Builder builder(*getModule());
             replaceCurrent(builder.makeSequence(
               builder.makeDrop(curr->condition), // might have side effects
@@ -1002,12 +1003,11 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
               } else {
                 // To use an if, the value must have no side effects, as in the
                 // if it may not execute.
-                FeatureSet features = getModule()->features;
-                if (!EffectAnalyzer(passOptions, features, br->value)
+                if (!EffectAnalyzer(passOptions, *getModule(), br->value)
                        .hasSideEffects()) {
                   // We also need to reorder the condition and the value.
                   if (EffectAnalyzer::canReorder(
-                        passOptions, features, br->condition, br->value)) {
+                        passOptions, *getModule(), br->condition, br->value)) {
                     ExpressionManipulator::nop(list[0]);
                     replaceCurrent(
                       builder.makeIf(br->condition, br->value, curr));
@@ -1045,9 +1045,9 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
                   // after ignoring the br_if.
                   list[0] = &nop;
                   auto canReorder = EffectAnalyzer::canReorder(
-                    passOptions, features, br->condition, curr);
+                    passOptions, *getModule(), br->condition, curr);
                   auto hasSideEffects =
-                    EffectAnalyzer(passOptions, features, curr)
+                    EffectAnalyzer(passOptions, *getModule(), curr)
                       .hasSideEffects();
                   list[0] = old;
                   if (canReorder && !hasSideEffects &&
@@ -1099,16 +1099,15 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
         }
         // Check if side effects allow this: we need to execute the two arms
         // unconditionally, and also to make the condition run last.
-        FeatureSet features = getModule()->features;
-        EffectAnalyzer ifTrue(passOptions, features, iff->ifTrue);
+        EffectAnalyzer ifTrue(passOptions, *getModule(), iff->ifTrue);
         if (ifTrue.hasSideEffects()) {
           return nullptr;
         }
-        EffectAnalyzer ifFalse(passOptions, features, iff->ifFalse);
+        EffectAnalyzer ifFalse(passOptions, *getModule(), iff->ifFalse);
         if (ifFalse.hasSideEffects()) {
           return nullptr;
         }
-        EffectAnalyzer condition(passOptions, features, iff->condition);
+        EffectAnalyzer condition(passOptions, *getModule(), iff->condition);
         if (condition.invalidates(ifTrue) || condition.invalidates(ifFalse)) {
           return nullptr;
         }
@@ -1363,7 +1362,7 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
           }
           // if the condition has side effects, we can't replace many
           // appearances of it with a single one
-          if (EffectAnalyzer(passOptions, getModule()->features, conditionValue)
+          if (EffectAnalyzer(passOptions, *getModule(), conditionValue)
                 .hasSideEffects()) {
             start++;
             continue;
diff --git a/src/passes/SimplifyGlobals.cpp b/src/passes/SimplifyGlobals.cpp
index 5a6337bd6..f162d50c5 100644
--- a/src/passes/SimplifyGlobals.cpp
+++ b/src/passes/SimplifyGlobals.cpp
@@ -108,8 +108,7 @@ struct GlobalUseScanner : public WalkerPass<PostWalker<GlobalUseScanner>> {
     }
 
     // See if reading a specific global is the only effect the condition has.
-    EffectAnalyzer condition(
-      getPassOptions(), getModule()->features, curr->condition);
+    EffectAnalyzer condition(getPassOptions(), *getModule(), curr->condition);
 
     if (condition.globalsRead.size() != 1) {
       return;
@@ -123,8 +122,7 @@ struct GlobalUseScanner : public WalkerPass<PostWalker<GlobalUseScanner>> {
     // See if writing the same global is the only effect the body has. (Note
     // that we don't need to care about the case where the body has no effects
     // at all - other pass would handle that trivial situation.)
-    EffectAnalyzer ifTrue(
-      getPassOptions(), getModule()->features, curr->ifTrue);
+    EffectAnalyzer ifTrue(getPassOptions(), *getModule(), curr->ifTrue);
     if (ifTrue.globalsWritten.size() != 1) {
       return;
     }
@@ -210,7 +208,7 @@ struct ConstantGlobalApplier
       return;
     }
     // Otherwise, invalidate if we need to.
-    EffectAnalyzer effects(getPassOptions(), getModule()->features);
+    EffectAnalyzer effects(getPassOptions(), *getModule());
     effects.visit(curr);
     assert(effects.globalsWritten.empty()); // handled above
     if (effects.calls) {
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 77b8f1a93..ee0459d7d 100644
--- a/src/passes/SimplifyLocals.cpp
+++ b/src/passes/SimplifyLocals.cpp
@@ -79,10 +79,8 @@ struct SimplifyLocals
     Expression** item;
     EffectAnalyzer effects;
 
-    SinkableInfo(Expression** item,
-                 PassOptions& passOptions,
-                 FeatureSet features)
-      : item(item), effects(passOptions, features, *item) {}
+    SinkableInfo(Expression** item, PassOptions& passOptions, Module& module)
+      : item(item), effects(passOptions, module, *item) {}
   };
 
   // a list of sinkables in a linear execution trace
@@ -315,7 +313,7 @@ struct SimplifyLocals
       }
     }
 
-    EffectAnalyzer effects(self->getPassOptions(), self->getModule()->features);
+    EffectAnalyzer effects(self->getPassOptions(), *self->getModule());
     if (effects.checkPre(curr)) {
       self->checkInvalidations(effects);
     }
@@ -401,8 +399,7 @@ struct SimplifyLocals
       }
     }
 
-    FeatureSet features = self->getModule()->features;
-    EffectAnalyzer effects(self->getPassOptions(), features);
+    EffectAnalyzer effects(self->getPassOptions(), *self->getModule());
     if (effects.checkPost(original)) {
       self->checkInvalidations(effects);
     }
@@ -411,7 +408,8 @@ struct SimplifyLocals
       Index index = set->index;
       assert(self->sinkables.count(index) == 0);
       self->sinkables.emplace(std::make_pair(
-        index, SinkableInfo(currp, self->getPassOptions(), features)));
+        index,
+        SinkableInfo(currp, self->getPassOptions(), *self->getModule())));
     }
 
     if (!allowNesting) {
@@ -428,7 +426,7 @@ struct SimplifyLocals
     // 'catch', because 'pop' should follow right after 'catch'.
     FeatureSet features = this->getModule()->features;
     if (features.hasExceptionHandling() &&
-        EffectAnalyzer(this->getPassOptions(), features, set->value)
+        EffectAnalyzer(this->getPassOptions(), *this->getModule(), set->value)
           .danglingPop) {
       return false;
     }
@@ -530,7 +528,6 @@ struct SimplifyLocals
     //   )
     //  )
     // so we must check for that.
-    FeatureSet features = this->getModule()->features;
     for (size_t j = 0; j < breaks.size(); j++) {
       // move break local.set's value to the break
       auto* breakLocalSetPointer = breaks[j].sinkables.at(sharedIndex).item;
@@ -548,8 +545,9 @@ struct SimplifyLocals
             Nop nop;
             *breakLocalSetPointer = &nop;
             EffectAnalyzer condition(
-              this->getPassOptions(), features, br->condition);
-            EffectAnalyzer value(this->getPassOptions(), features, set);
+              this->getPassOptions(), *this->getModule(), br->condition);
+            EffectAnalyzer value(
+              this->getPassOptions(), *this->getModule(), set);
             *breakLocalSetPointer = set;
             if (condition.invalidates(value)) {
               // indeed, we can't do this, stop
diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp
index 14a464297..dec4add01 100644
--- a/src/passes/Vacuum.cpp
+++ b/src/passes/Vacuum.cpp
@@ -69,7 +69,6 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
   // i32.eqz returns the same type as it receives. But for an expression that
   // returns a different type, if the type matters then we cannot replace it.
   Expression* optimize(Expression* curr, bool resultUsed, bool typeMatters) {
-    FeatureSet features = getModule()->features;
     auto type = curr->type;
     // If the type is none, then we can never replace it with another type.
     if (type == Type::none) {
@@ -102,7 +101,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
         return curr;
       }
       // Check if this expression itself has side effects, ignoring children.
-      EffectAnalyzer self(getPassOptions(), features);
+      EffectAnalyzer self(getPassOptions(), *getModule());
       self.visit(curr);
       if (self.hasSideEffects()) {
         return curr;
@@ -111,7 +110,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
       // get rid of it. However, the children may have side effects.
       SmallVector<Expression*, 1> childrenWithEffects;
       for (auto* child : ChildIterator(curr)) {
-        if (EffectAnalyzer(getPassOptions(), features, child)
+        if (EffectAnalyzer(getPassOptions(), *getModule(), child)
               .hasSideEffects()) {
           childrenWithEffects.push_back(child);
         }
@@ -292,7 +291,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
     // Note that we check the type here to avoid removing unreachable code - we
     // leave that for DCE.
     if (curr->type == Type::none &&
-        !EffectAnalyzer(getPassOptions(), getModule()->features, curr)
+        !EffectAnalyzer(getPassOptions(), *getModule(), curr)
            .hasUnremovableSideEffects()) {
       ExpressionManipulator::nop(curr);
       return;
@@ -361,8 +360,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
   void visitTry(Try* curr) {
     // If try's body does not throw, the whole try-catch can be replaced with
     // the try's body.
-    if (!EffectAnalyzer(getPassOptions(), getModule()->features, curr->body)
-           .throws) {
+    if (!EffectAnalyzer(getPassOptions(), *getModule(), curr->body).throws) {
       replaceCurrent(curr->body);
       for (auto* catchBody : curr->catchBodies) {
         typeUpdater.noteRecursiveRemoval(catchBody);
@@ -379,7 +377,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
       ExpressionManipulator::nop(curr->body);
     }
     if (curr->getResults() == Type::none &&
-        !EffectAnalyzer(getPassOptions(), getModule()->features, curr->body)
+        !EffectAnalyzer(getPassOptions(), *getModule(), curr->body)
            .hasSideEffects()) {
       ExpressionManipulator::nop(curr->body);
     }
diff --git a/src/wasm2js.h b/src/wasm2js.h
index ad8e32647..3e264f98b 100644
--- a/src/wasm2js.h
+++ b/src/wasm2js.h
@@ -1298,12 +1298,11 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m,
       // If the target has effects that interact with the operands, we must
       // reorder it to the start.
       bool mustReorder = false;
-      EffectAnalyzer targetEffects(
-        parent->options, module->features, curr->target);
+      EffectAnalyzer targetEffects(parent->options, *module, curr->target);
       if (targetEffects.hasAnything()) {
         for (auto* operand : curr->operands) {
           if (targetEffects.invalidates(
-                EffectAnalyzer(parent->options, module->features, operand))) {
+                EffectAnalyzer(parent->options, *module, operand))) {
             mustReorder = true;
             break;
           }
@@ -1930,11 +1929,9 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m,
       // side effects, as a JS conditional does not visit both sides.
       bool useLocals = false;
       EffectAnalyzer conditionEffects(
-        parent->options, module->features, curr->condition);
-      EffectAnalyzer ifTrueEffects(
-        parent->options, module->features, curr->ifTrue);
-      EffectAnalyzer ifFalseEffects(
-        parent->options, module->features, curr->ifFalse);
+        parent->options, *module, curr->condition);
+      EffectAnalyzer ifTrueEffects(parent->options, *module, curr->ifTrue);
+      EffectAnalyzer ifFalseEffects(parent->options, *module, curr->ifFalse);
       if (conditionEffects.invalidates(ifTrueEffects) ||
           conditionEffects.invalidates(ifFalseEffects) ||
           ifTrueEffects.hasSideEffects() || ifFalseEffects.hasSideEffects()) {