Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify regression in #560 #580

Open
dsharlet opened this issue Jan 31, 2025 · 0 comments
Open

Simplify regression in #560 #580

dsharlet opened this issue Jan 31, 2025 · 0 comments

Comments

@dsharlet
Copy link
Owner

#560 has a change reverted by the following diff:

diff --git a/builder/simplify.cc b/builder/simplify.cc
index aeaa156..84e9555 100644
--- a/builder/simplify.cc
+++ b/builder/simplify.cc
@@ -1239,7 +1239,7 @@ public:
     if (auto cstep = as_constant(step)) alignment.modulus = *cstep;
     if (auto cmin = as_constant(bounds.min)) alignment.remainder = *cmin;
     // If we're in the body of the loop, then we know that bounds.max >= bounds.min.
-    stmt body = mutate_with_bounds(op->body, op->sym, {bounds.min, mutate(max(bounds.min, bounds.max))}, alignment);
+    stmt body = mutate_with_bounds(op->body, op->sym, {bounds.min, bounds.max}, alignment);
     for (auto& i : buffers) {
       if (i) --i->loop_depth;
     }

This change slightly regressed the simplification of this example:

SLINKY_VERBOSE=1 bazel run --config=ubsan builder/test:pipeline -- --gtest_filter=split_schedule_mode/elementwise.pipeline_2d/2147483647_1_0

from producing:

out.y = loop(parallel, [buffer_min(out, 1), buffer_max(out, 1)], 1) {
 closure {in, out, out.y} in {
  out.out.y = crop_dim(out, 1, [out.y, out.y]) {
   intm.out.y = crop_dim(out, 1, [out.y, out.y]) {
    out.x = loop(parallel, [buffer_min(out, 0), buffer_max(out, 0)], 1) {
     closure {in, out, out.x, out.out.y, intm.out.y} in {
      intm.out.x = crop_dim(intm.out.y, 0, [out.x, out.x]) {
       call(<anonymous target>, {in}, {intm.out.x})
      }
      out.out.x = crop_dim(out.out.y, 0, [out.x, out.x]) {
       call(<anonymous target>, {out}, {out.out.x})
      }
     }
    }
   }
  }
 }
}

vs.

out.y = loop(parallel, [buffer_min(out, 1), buffer_max(out, 1)], 1) {
 closure {in, out, out.y} in {
  out.out.y = crop_dim(out, 1, [out.y, out.y]) {
   intm.out.y = crop_dim(out, 1, [out.y, out.y]) {
    out.x = loop(parallel, [buffer_min(out, 0), buffer_max(out, 0)], 1) {
     closure {in, out, out.x, out.out.y, intm.out.y} in {
      out.out.x = crop_dim(out.out.y, 0, [out.x, out.x]) {
       intm.out.x = crop_dim(intm.out.y, 0, [out.x, buffer_max(out.out.x, 0)]) {
        call(<anonymous target>, {in}, {intm.out.x})
       }
       call(<anonymous target>, {out}, {out.out.x})
      }
     }
    }
   }
  }
 }
}

Note the extra nesting of the intm.out.x = crop_dim(...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant