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

Add missing subsume uses in egraph rules #7879

Merged
merged 2 commits into from
Feb 6, 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: 6 additions & 0 deletions cranelift/codegen/src/opts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ of it boils down to the fact that, unlike traditional e-graphs, our rules are
place of `x` in such cases would introduce uses of `y` where it is not
defined.

An exception to this rule is discarding constants, as they can be
rematerialized anywhere without introducing correctness issues. For example,
the (admittedly silly) rule `(select 1 x (iconst_u _)) => x` would be a good
candidate for not using `subsume`, as it does not discard any non-constant
values introduced in its LHS.

3. Avoid overly general rewrites like commutativity and associativity. Instead,
prefer targeted instances of the rewrite (for example, canonicalizing adds
where one operand is a constant such that the constant is always the add's
Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/opts/extends.isle
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
(slt ty
(uextend $I64 x @ (value_type $I32))
(iconst_u _ 0)))
(iconst_u ty 0))
(subsume (iconst_u ty 0)))
(rule (simplify
(sge ty
(uextend $I64 x @ (value_type $I32))
(iconst_u _ 0)))
(iconst_u ty 1))
(subsume (iconst_u ty 1)))

;; Sign-extending can't change whether a number is zero nor how it signed-compares to zero
(rule (simplify (eq _ (sextend _ x@(value_type ty)) (iconst_s _ 0)))
Expand Down
20 changes: 10 additions & 10 deletions cranelift/codegen/src/opts/icmp.isle
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

;; `x == x` is always true for integers; `x != x` is false. Strict
;; inequalities are false, and loose inequalities are true.
(rule (simplify (eq (ty_int ty) x x)) (iconst_u ty 1))
(rule (simplify (ne (ty_int ty) x x)) (iconst_u ty 0))
(rule (simplify (ugt (ty_int ty) x x)) (iconst_u ty 0))
(rule (simplify (uge (ty_int ty) x x)) (iconst_u ty 1))
(rule (simplify (sgt (ty_int ty) x x)) (iconst_u ty 0))
(rule (simplify (sge (ty_int ty) x x)) (iconst_u ty 1))
(rule (simplify (ult (ty_int ty) x x)) (iconst_u ty 0))
(rule (simplify (ule (ty_int ty) x x)) (iconst_u ty 1))
(rule (simplify (slt (ty_int ty) x x)) (iconst_u ty 0))
(rule (simplify (sle (ty_int ty) x x)) (iconst_u ty 1))
(rule (simplify (eq (ty_int ty) x x)) (subsume (iconst_u ty 1)))
(rule (simplify (ne (ty_int ty) x x)) (subsume (iconst_u ty 0)))
(rule (simplify (ugt (ty_int ty) x x)) (subsume (iconst_u ty 0)))
(rule (simplify (uge (ty_int ty) x x)) (subsume (iconst_u ty 1)))
(rule (simplify (sgt (ty_int ty) x x)) (subsume (iconst_u ty 0)))
(rule (simplify (sge (ty_int ty) x x)) (subsume (iconst_u ty 1)))
(rule (simplify (ult (ty_int ty) x x)) (subsume (iconst_u ty 0)))
(rule (simplify (ule (ty_int ty) x x)) (subsume (iconst_u ty 1)))
(rule (simplify (slt (ty_int ty) x x)) (subsume (iconst_u ty 0)))
(rule (simplify (sle (ty_int ty) x x)) (subsume (iconst_u ty 1)))

;; Optimize icmp-of-icmp.
(rule (simplify (ne ty
Expand Down