From 268f6bfc1d383efe6793907c9d26209df2e96c94 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Tue, 31 Jan 2023 18:53:23 -0800 Subject: [PATCH] Revert "Cranelift: Rewrite `or(and(x, y), not(y)) => or(x, not(y))` (#5676)" (#5682) This reverts commit 8c9eb9939bfbe634aa80097b02f34068549e804b. Fixes #5680 --- cranelift/codegen/src/isle_prelude.rs | 5 --- cranelift/codegen/src/opts/algebraic.isle | 27 ------------ cranelift/codegen/src/prelude.isle | 3 -- .../filetests/filetests/egraph/algebraic.clif | 44 +------------------ .../runtests/or-and-y-with-not-y.clif | 34 -------------- 5 files changed, 1 insertion(+), 112 deletions(-) delete mode 100644 cranelift/filetests/filetests/runtests/or-and-y-with-not-y.clif diff --git a/cranelift/codegen/src/isle_prelude.rs b/cranelift/codegen/src/isle_prelude.rs index 3d5a7f3bdb..a32cee62c0 100644 --- a/cranelift/codegen/src/isle_prelude.rs +++ b/cranelift/codegen/src/isle_prelude.rs @@ -109,11 +109,6 @@ macro_rules! isle_common_prelude_methods { !x } - #[inline] - fn u64_eq(&mut self, x: u64, y: u64) -> u64 { - u64::from(x == y) - } - #[inline] fn u64_is_zero(&mut self, value: u64) -> bool { 0 == value diff --git a/cranelift/codegen/src/opts/algebraic.isle b/cranelift/codegen/src/opts/algebraic.isle index 1369cbfab0..64a64a3a1f 100644 --- a/cranelift/codegen/src/opts/algebraic.isle +++ b/cranelift/codegen/src/opts/algebraic.isle @@ -138,33 +138,6 @@ (rule (simplify (bnot ty (band t x y))) (bor ty (bnot ty x) (bnot ty y))) -;; `or(and(x, y), not(y)) == or(x, not(y))` -(rule (simplify (bor ty - (band ty x y) - z @ (bnot ty y))) - (bor ty x z)) -;; Duplicate the rule but swap the `bor` operands because `bor` is -;; commutative. We could, of course, add a `simplify` rule to do the commutative -;; swap for all `bor`s but this will bloat the e-graph with many e-nodes. It is -;; cheaper to have additional rules, rather than additional e-nodes, because we -;; amortize their cost via ISLE's smart codegen. -(rule (simplify (bor ty - z @ (bnot ty y) - (band ty x y))) - (bor ty x z)) - -;; `or(and(x, y), not(y)) == or(x, not(y))` specialized for constants, since -;; otherwise we may not know that `z == not(y)` since we don't generally expand -;; constants in the e-graph. -;; -;; (No need to duplicate for commutative `bor` for this constant version because -;; we move constants to the right.) -(rule (simplify (bor ty - (band ty x (iconst ty (u64_from_imm64 y))) - z @ (iconst ty (u64_from_imm64 zk)))) - (if (u64_eq zk (u64_not y))) - (bor ty x z)) - ;; x*2 == 2*x == x+x. (rule (simplify (imul ty x (iconst _ (simm32 2)))) (iadd ty x x)) diff --git a/cranelift/codegen/src/prelude.isle b/cranelift/codegen/src/prelude.isle index fea0eee2f9..c1e7c8d2b8 100644 --- a/cranelift/codegen/src/prelude.isle +++ b/cranelift/codegen/src/prelude.isle @@ -141,9 +141,6 @@ (decl pure u64_not (u64) u64) (extern constructor u64_not u64_not) -(decl pure u64_eq (u64 u64) u64) -(extern constructor u64_eq u64_eq) - (decl pure u64_sextend_u32 (u64) u64) (extern constructor u64_sextend_u32 u64_sextend_u32) diff --git a/cranelift/filetests/filetests/egraph/algebraic.clif b/cranelift/filetests/filetests/egraph/algebraic.clif index 54a786b466..33e83936c7 100644 --- a/cranelift/filetests/filetests/egraph/algebraic.clif +++ b/cranelift/filetests/filetests/egraph/algebraic.clif @@ -40,7 +40,7 @@ block0(v0: i32): return v3 ; check: v4 = iconst.i32 0xffff_ffe0 ; check: v5 = band v0, v4 - ; check: return v5 + ; return v5 } function %unsigned_shift_right_shift_left_i64(i64) -> i64 { @@ -86,45 +86,3 @@ block0(v0: i64): ; check: v5 = band v0, v4 ; return v5 } - -function %or_and_y_with_not_y(i8, i8) -> i8 { -block0(v0: i8, v1: i8): - v2 = band v0, v1 - v3 = bnot v1 - v4 = bor v2, v3 - return v4 - ; check: v5 = bor v0, v3 - ; check: return v5 -} - -function %or_and_constant_with_not_constant(i8) -> i8 { -block0(v0: i8): - v1 = iconst.i8 -4 - v2 = band v0, v1 - v3 = iconst.i8 3 - v4 = bor v2, v3 - return v4 - ; check: v5 = bor v0, v3 - ; check: return v5 -} - -function %or_and_y_with_not_y(i8, i8) -> i8 { -block0(v0: i8, v1: i8): - v2 = band v0, v1 - v3 = bnot v1 - v4 = bor v3, v2 - return v4 - ; check: v5 = bor v0, v3 - ; check: return v5 -} - -function %or_and_constant_with_not_constant(i8) -> i8 { -block0(v0: i8): - v1 = iconst.i8 -4 - v2 = band v0, v1 - v3 = iconst.i8 3 - v4 = bor v3, v2 - return v4 - ; check: v6 = bor v0, v3 - ; check: return v6 -} diff --git a/cranelift/filetests/filetests/runtests/or-and-y-with-not-y.clif b/cranelift/filetests/filetests/runtests/or-and-y-with-not-y.clif deleted file mode 100644 index 55532d23bc..0000000000 --- a/cranelift/filetests/filetests/runtests/or-and-y-with-not-y.clif +++ /dev/null @@ -1,34 +0,0 @@ -;; Test the rewrite: `or(and(x, y), not(y)) => or(x, not(y))` - -test interpret -test run -target aarch64 -target x86_64 -target riscv64 -target s390x - -function %or_and_y_with_not_y(i8, i8) -> i8 { -block0(v0: i8, v1: i8): - v2 = band v0, v1 - v3 = bnot v1 - v4 = bor v2, v3 - return v4 -} -; run: %or_and_y_with_not_y(0xff, 0x0a) == 0xff -; run: %or_and_y_with_not_y(0xff, 0xb0) == 0xff -; run: %or_and_y_with_not_y(0xaa, 0x0a) == 0xff -; run: %or_and_y_with_not_y(0xaa, 0xb0) == 0xef -; run: %or_and_y_with_not_y(0x00, 0x0a) == 0xf5 -; run: %or_and_y_with_not_y(0x00, 0xb0) == 0x4f - -function %or_and_constant_with_not_constant(i8) -> i8 { -block0(v0: i8): - v1 = iconst.i8 -4 - v2 = band v0, v1 - v3 = iconst.i8 3 - v4 = bor v2, v3 - return v4 -} -; run: %or_and_constant_with_not_constant(0xff) == 0xff -; run: %or_and_constant_with_not_constant(0xaa) == 0xab -; run: %or_and_constant_with_not_constant(0x00) == 0x03