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

Track whether or not let expressions failed to solve in solver #7982

Merged
merged 14 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/ModulusRemainder.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#include <cstdint>

#include "Util.h"

namespace Halide {

struct Expr;
Expand Down Expand Up @@ -83,8 +85,8 @@ ModulusRemainder modulus_remainder(const Expr &e, const Scope<ModulusRemainder>
/** Reduce an expression modulo some integer. Returns true and assigns
* to remainder if an answer could be found. */
///@{
bool reduce_expr_modulo(const Expr &e, int64_t modulus, int64_t *remainder);
bool reduce_expr_modulo(const Expr &e, int64_t modulus, int64_t *remainder, const Scope<ModulusRemainder> &scope);
HALIDE_MUST_USE_RESULT bool reduce_expr_modulo(const Expr &e, int64_t modulus, int64_t *remainder);
HALIDE_MUST_USE_RESULT bool reduce_expr_modulo(const Expr &e, int64_t modulus, int64_t *remainder, const Scope<ModulusRemainder> &scope);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"must use result" should really be the default in the language, with the current behavior opt-in :-/

///@}

void modulus_remainder_test();
Expand Down
35 changes: 26 additions & 9 deletions src/Solve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,22 @@ class SolveExpression : public IRMutator {
map<Expr, CacheEntry, ExprCompare>::iterator iter = cache.find(e);
if (iter == cache.end()) {
// Not in the cache, call the base class version.
debug(4) << "Mutating " << e << " (" << uses_var << ")\n";
debug(4) << "Mutating " << e << " (" << uses_var << ", " << failed << ")\n";
bool old_uses_var = uses_var;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sure which C++ had a terse way to express this pattern (that wasn't an RIAA class)...

uses_var = false;
bool old_failed = failed;
failed = false;
Expr new_e = IRMutator::mutate(e);
CacheEntry entry = {new_e, uses_var};
CacheEntry entry = {new_e, uses_var, failed};
uses_var = old_uses_var || uses_var;
failed = old_failed || failed;
cache[e] = entry;
debug(4) << "(Miss) Rewrote " << e << " -> " << new_e << " (" << uses_var << ")\n";
debug(4) << "(Miss) Rewrote " << e << " -> " << new_e << " (" << uses_var << ", " << failed << ")\n";
return new_e;
} else {
// Cache hit.
uses_var = uses_var || iter->second.uses_var;
failed = failed || iter->second.failed;
debug(4) << "(Hit) Rewrote " << e << " -> " << iter->second.expr << " (" << uses_var << ")\n";
return iter->second.expr;
}
Expand All @@ -75,7 +79,7 @@ class SolveExpression : public IRMutator {
// stateless, so we can cache everything.
struct CacheEntry {
Expr expr;
bool uses_var;
bool uses_var, failed;
};
map<Expr, CacheEntry, ExprCompare> cache;

Expand Down Expand Up @@ -388,16 +392,25 @@ class SolveExpression : public IRMutator {
const Mul *mul_a = a.as<Mul>();
Expr expr;
if (a_uses_var && !b_uses_var) {
const int64_t *ib = as_const_int(b);
auto is_multiple_of_b = [&](const Expr &e) {
if (ib) {
int64_t r = 0;
return reduce_expr_modulo(e, *ib, &r) && r == 0;
} else {
return can_prove(e / b * b == e);
}
};
if (add_a && !a_failed &&
can_prove(add_a->a / b * b == add_a->a)) {
is_multiple_of_b(add_a->a)) {
// (f(x) + a) / b -> f(x) / b + a / b
expr = mutate(simplify(add_a->a / b) + add_a->b / b);
} else if (sub_a && !a_failed &&
can_prove(sub_a->a / b * b == sub_a->a)) {
is_multiple_of_b(sub_a->a)) {
// (f(x) - a) / b -> f(x) / b - a / b
expr = mutate(simplify(sub_a->a / b) - sub_a->b / b);
} else if (mul_a && !a_failed && no_overflow_int(op->type) &&
can_prove(mul_a->b / b * b == mul_a->b)) {
is_multiple_of_b(mul_a->b)) {
// (f(x) * a) / b -> f(x) * (a / b)
expr = mutate(mul_a->a * (mul_a->b / b));
}
Expand Down Expand Up @@ -776,6 +789,7 @@ class SolveExpression : public IRMutator {
} else if (scope.contains(op->name)) {
CacheEntry e = scope.get(op->name);
uses_var = uses_var || e.uses_var;
failed = failed || e.failed;
return e.expr;
} else if (external_scope.contains(op->name)) {
Expr e = external_scope.get(op->name);
Expand All @@ -790,11 +804,14 @@ class SolveExpression : public IRMutator {

Expr visit(const Let *op) override {
bool old_uses_var = uses_var;
bool old_failed = failed;
uses_var = false;
failed = false;
Expr value = mutate(op->value);
CacheEntry e = {value, uses_var};

CacheEntry e = {value, uses_var, failed};
uses_var = old_uses_var;
failed = old_failed;

ScopedBinding<CacheEntry> bind(scope, op->name, e);
return mutate(op->body);
}
Expand Down
Loading