Remove the simplifier's ability to preserve dead letstmts (#8899)

Remove the simplifiers ability to preserve dead letstmts

We shouldn't need it anymore thanks to .loop_max symbols now being
regular lets instead of magic things in need of preservation.

Author: abadams

src/BoundConstantExtentLoops.cpp

@@ -67,7 +67,6 @@ class BoundLoops : public IRMutator {
                 extent = remove_likelies(extent);
                 extent = substitute_in_all_lets(extent);
                 extent = simplify(extent,
-                                  true,
                                   Scope<Interval>::empty_scope(),
                                   Scope<ModulusRemainder>::empty_scope(),
                                   facts);

src/LowerWarpShuffles.cpp

@@ -76,8 +76,8 @@ Expr reduce_expr_helper(Expr e, const Expr &modulus) {
 }
 
 Expr reduce_expr(Expr e, const Expr &modulus, const Scope<Interval> &bounds) {
-    e = reduce_expr_helper(simplify(e, true, bounds), modulus);
-    if (is_const_one(simplify(e >= 0 && e < modulus, true, bounds))) {
+    e = reduce_expr_helper(simplify(e, bounds), modulus);
+    if (is_const_one(simplify(e >= 0 && e < modulus, bounds))) {
         return e;
     } else {
         return e % modulus;
@@ -285,7 +285,7 @@ class DetermineAllocStride : public IRVisitor {
 
     // A version of can_prove which exploits the constant bounds we've been tracking
     bool can_prove(const Expr &e) {
-        return is_const_one(simplify(e, true, bounds));
+        return is_const_one(simplify(e, bounds));
     }
 
     Expr get_stride() {
@@ -411,7 +411,7 @@ class LowerWarpShuffles : public IRMutator {
                 // the number of lanes (rounded up).
                 Expr extent = op->extent();
                 Expr new_size = (alloc->extents[0] + extent - 1) / extent;
-                new_size = simplify(new_size, true, bounds);
+                new_size = simplify(new_size, bounds);
                 new_size = find_constant_bound(new_size, Direction::Upper, bounds);
                 auto sz = as_const_int(new_size);
                 user_assert(sz) << "Warp-level allocation with non-constant size: "
@@ -511,7 +511,7 @@ class LowerWarpShuffles : public IRMutator {
             // of the index and shifting the high bits down to cover
             // them. Reassembling the result into a flat address gives
             // the expression below.
-            Expr in_warp_idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), true, bounds);
+            Expr in_warp_idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), bounds);
             return Store::make(op->name, value, in_warp_idx, op->param, op->predicate, ModulusRemainder());
         } else {
             return IRMutator::visit(op);
@@ -536,7 +536,7 @@ class LowerWarpShuffles : public IRMutator {
                 // Load the right lanes from stripe number i
                 equiv = select(idx >= i, make_warp_load(type, name, make_const(idx.type(), i), lane), equiv);
             }
-            return simplify(equiv, true, bounds);
+            return simplify(equiv, bounds);
         }
 
         // Load the value to be shuffled
@@ -606,7 +606,7 @@ class LowerWarpShuffles : public IRMutator {
         } else if (expr_match((this_lane + wild) % wild, lane, result) &&
                    (bits = is_const_power_of_two_integer(result[1])) &&
                    *bits <= 5) {
-            result[0] = simplify(result[0] % result[1], true, bounds);
+            result[0] = simplify(result[0] % result[1], bounds);
             // Rotate. Mux a shuffle up and a shuffle down. Uses fewer
             // intermediate registers than using a general gather for
             // this.
@@ -617,7 +617,7 @@ class LowerWarpShuffles : public IRMutator {
                                  shfl_args({membermask, base_val, (1 << *bits) - result[0], 0}), Call::PureExtern);
             Expr cond = (this_lane >= (1 << *bits) - result[0]);
             Expr equiv = select(cond, up, down);
-            shuffled = simplify(equiv, true, bounds);
+            shuffled = simplify(equiv, bounds);
         } else {
             // The format of the mask is a pain. The high bits tell
             // you how large the a warp is for this instruction
@@ -647,10 +647,10 @@ class LowerWarpShuffles : public IRMutator {
             Expr stride = alloc->stride;
 
             // Break the index into lane and stripe components
-            Expr lane = simplify(reduce_expr(idx / stride, warp_size, bounds), true, bounds);
-            idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), true, bounds);
+            Expr lane = simplify(reduce_expr(idx / stride, warp_size, bounds), bounds);
+            idx = simplify((idx / (warp_size * stride)) * stride + reduce_expr(idx, stride, bounds), bounds);
             // We don't want the idx to depend on the lane var, so try to eliminate it
-            idx = simplify(solve_expression(idx, this_lane_name).result, true, bounds);
+            idx = simplify(solve_expression(idx, this_lane_name).result, bounds);
             return make_warp_load(op->type, op->name, idx, lane);
         } else {
             return IRMutator::visit(op);

src/ParallelRVar.cpp

@@ -157,14 +157,9 @@ bool can_parallelize_rvar(const string &v,
     // Pull out common non-boolean terms
     hazard = common_subexpression_elimination(hazard);
     hazard = SubstituteInBooleanLets().mutate(hazard);
-    hazard = simplify(hazard, false, bounds);
+    hazard = simplify(hazard, bounds);
     debug(3) << "Simplified to: " << hazard << "\n";
 
-    // strip lets
-    while (const Let *l = hazard.as<Let>()) {
-        hazard = l->body;
-    }
-
     return is_const_zero(hazard);
 }
 

src/PrintLoopNest.cpp

@@ -216,7 +216,7 @@ string print_loop_nest(const vector<Function> &output_funcs) {
     s = allocation_bounds_inference(s, env, func_bounds);
     s = remove_undef(s);
     s = uniquify_variable_names(s);
-    s = simplify(s, false);
+    s = simplify(s);
 
     // Now convert that to pseudocode
     std::ostringstream sstr;

src/Simplify.cpp

@@ -15,8 +15,7 @@ using std::pair;
 using std::string;
 using std::vector;
 
-Simplify::Simplify(bool r, const Scope<Interval> *bi, const Scope<ModulusRemainder> *ai)
-    : remove_dead_code(r) {
+Simplify::Simplify(const Scope<Interval> *bi, const Scope<ModulusRemainder> *ai) {
 
     // Only respect the constant bounds from the containing scope.
     for (auto iter = bi->cbegin(); iter != bi->cend(); ++iter) {
@@ -361,11 +360,11 @@ Simplify::ScopedFact::~ScopedFact() {
     }
 }
 
-Expr simplify(const Expr &e, bool remove_dead_let_stmts,
+Expr simplify(const Expr &e,
               const Scope<Interval> &bounds,
               const Scope<ModulusRemainder> &alignment,
               const std::vector<Expr> &assumptions) {
-    Simplify m(remove_dead_let_stmts, &bounds, &alignment);
+    Simplify m(&bounds, &alignment);
     std::vector<Simplify::ScopedFact> facts;
     facts.reserve(assumptions.size());
     for (const Expr &a : assumptions) {
@@ -378,11 +377,11 @@ Expr simplify(const Expr &e, bool remove_dead_let_stmts,
     return result;
 }
 
-Stmt simplify(const Stmt &s, bool remove_dead_let_stmts,
+Stmt simplify(const Stmt &s,
               const Scope<Interval> &bounds,
               const Scope<ModulusRemainder> &alignment,
               const std::vector<Expr> &assumptions) {
-    Simplify m(remove_dead_let_stmts, &bounds, &alignment);
+    Simplify m(&bounds, &alignment);
     std::vector<Simplify::ScopedFact> facts;
     facts.reserve(assumptions.size());
     for (const Expr &a : assumptions) {
@@ -416,7 +415,7 @@ bool can_prove(Expr e, const Scope<Interval> &bounds) {
 
     Expr orig = e;
 
-    e = simplify(e, true, bounds);
+    e = simplify(e, bounds);
 
     // Take a closer look at all failed proof attempts to hunt for
     // simplifier weaknesses

src/Simplify.h

@@ -22,12 +22,10 @@ namespace Internal {
  */
 // @{
 Stmt simplify(const Stmt &,
-              bool remove_dead_code = true,
               const Scope<Interval> &bounds = Scope<Interval>::empty_scope(),
               const Scope<ModulusRemainder> &alignment = Scope<ModulusRemainder>::empty_scope(),
               const std::vector<Expr> &assumptions = std::vector<Expr>());
 Expr simplify(const Expr &,
-              bool remove_dead_code = true,
               const Scope<Interval> &bounds = Scope<Interval>::empty_scope(),
               const Scope<ModulusRemainder> &alignment = Scope<ModulusRemainder>::empty_scope(),
               const std::vector<Expr> &assumptions = std::vector<Expr>());

src/Simplify_Internal.h

@@ -34,7 +34,7 @@ class Simplify : public VariadicVisitor<Simplify, Expr, Stmt> {
     using Super = VariadicVisitor<Simplify, Expr, Stmt>;
 
 public:
-    Simplify(bool r, const Scope<Interval> *bi, const Scope<ModulusRemainder> *ai);
+    Simplify(const Scope<Interval> *bi, const Scope<ModulusRemainder> *ai);
 
     struct ExprInfo {
         // We track constant integer bounds when they exist
@@ -353,8 +353,6 @@ class Simplify : public VariadicVisitor<Simplify, Expr, Stmt> {
     }
 #endif
 
-    bool remove_dead_code;
-
     // Returns true iff t is an integral type where overflow is undefined
     HALIDE_ALWAYS_INLINE
     bool no_overflow_int(Type t) {

src/Simplify_Let.cpp

@@ -293,8 +293,7 @@ Body Simplify::simplify_let(const LetOrLetStmt *op, ExprInfo *info) {
             find_var_uses(frame.new_value, unused_vars);
         }
 
-        if ((!remove_dead_code && std::is_same_v<LetOrLetStmt, LetStmt>) ||
-            (frame.info.old_uses > 0 && !unused_vars.count(frame.op->name))) {
+        if (frame.info.old_uses > 0 && !unused_vars.count(frame.op->name)) {
             // The old name is still in use. We'd better keep it as well.
             result = LetOrLetStmt::make(frame.op->name, frame.value, result);
             find_var_uses(frame.value, unused_vars);

src/Simplify_Stmts.cpp

@@ -79,9 +79,13 @@ Stmt Simplify::visit(const IfThenElse *op) {
         else_case = Stmt();
     }
 
-    // Pull out common nodes, but only when the "late in lowering" flag is set. This
-    // avoids simplifying specializations before they have a chance to specialize.
-    if (remove_dead_code && equal(then_case, else_case)) {
+    // This code used to use the remove_dead_lets flag to not merge equal
+    // clauses on the grounds that they might be specializations that will
+    // simplify later. However, specializations should be simplified
+    // aggressively quite early in lowering. If in future there is a bug with
+    // specializations seemingly disappearing halfway through lowering, try
+    // disabling this.
+    if (equal(then_case, else_case)) {
         return then_case;
     }
     const Acquire *then_acquire = then_case.as<Acquire>();

src/StorageFolding.cpp

@@ -588,14 +588,14 @@ class AttemptStorageFoldingOfFunction : public IRMutator {
             Expr loop_var = Variable::make(Int(32), op->name);
             Expr steady_state = (op->min < loop_var);
 
-            Expr min_steady = simplify(substitute(steady_state, const_true(), min), true, steady_bounds);
-            Expr max_steady = simplify(substitute(steady_state, const_true(), max), true, steady_bounds);
-            Expr min_initial = simplify(substitute(steady_state, const_false(), min), true, bounds);
-            Expr max_initial = simplify(substitute(steady_state, const_false(), max), true, bounds);
-            Expr extent_initial = simplify(substitute(loop_var, op->min, max_initial - min_initial + 1), true, bounds);
-            Expr extent_steady = simplify(max_steady - min_steady + 1, true, steady_bounds);
+            Expr min_steady = simplify(substitute(steady_state, const_true(), min), steady_bounds);
+            Expr max_steady = simplify(substitute(steady_state, const_true(), max), steady_bounds);
+            Expr min_initial = simplify(substitute(steady_state, const_false(), min), bounds);
+            Expr max_initial = simplify(substitute(steady_state, const_false(), max), bounds);
+            Expr extent_initial = simplify(substitute(loop_var, op->min, max_initial - min_initial + 1), bounds);
+            Expr extent_steady = simplify(max_steady - min_steady + 1, steady_bounds);
             Expr extent = Max::make(extent_initial, extent_steady);
-            extent = simplify(common_subexpression_elimination(extent), true, bounds);
+            extent = simplify(common_subexpression_elimination(extent), bounds);
 
             // Find the StorageDim corresponding to dim.
             const std::vector<StorageDim> &storage_dims = func.schedule().storage_dims();