Skip to content

Commit 34a34a8

Browse files
elliotttjameysharplpereiracfallin
authored
Backport #8005 to release-17.0.0 (#8007)
* cranelift: Fix ireduce rules (#8005) We had two optimization rules which started off like this: (rule (simplify (ireduce smallty val@(binary_op _ op x y))) (if-let _ (reducible_modular_op val)) ...) This was intended to check that `x` and `y` came from an instruction which not only was a binary op but also matched `reducible_modular_op`. Unfortunately, both `binary_op` and `reducible_modular_op` were multi-terms. - So `binary_op` would search the eclass rooted at `val` to find each instruction that uses a binary operator. - Then `reducible_modular_op` would search the entire eclass again to find an instruction which matched its criteria. Nothing ensured that both searches would find the same instruction. The reason these rules were written this way was because they had additional guards (`will_simplify_with_ireduce`) which made them fairly complex, and it seemed desirable to not have to copy those guards for every operator where we wanted to apply this optimization. However, we've decided that checking whether the rule is actually an improvement is not desirable. In general, that should be the job of the cost function. Blindly adding equivalent expressions gives us more opportunities for other rules to fire, and we have global recursion and growth limits to keep the process from going too wild. As a result, we can just delete those guards. That allows us to write the rules in a more straightforward way. Fixes #7999. Co-authored-by: Trevor Elliott <telliott@fastly.com> Co-authored-by: L Pereira <l.pereira@fastly.com> Co-authored-by: Chris Fallin <chris@cfallin.org> * Update RELEASES.md --------- Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: L Pereira <l.pereira@fastly.com> Co-authored-by: Chris Fallin <chris@cfallin.org>
1 parent 601e229 commit 34a34a8

File tree

3 files changed

+20
-55
lines changed

3 files changed

+20
-55
lines changed

RELEASES.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
--------------------------------------------------------------------------------
22

3+
## 17.0.2
4+
5+
### Fixed
6+
7+
* Fix an egraph rule bug that was permitting an incorrect `ireduce` rewrite to
8+
unary and binary operations, leading to miscompilations.
9+
[#8005](https://github.com/bytecodealliance/wasmtime/pull/8005)
10+
11+
--------------------------------------------------------------------------------
12+
313
## 17.0.1
414

515
### Fixed

cranelift/codegen/src/opts/extends.isle

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -75,50 +75,17 @@
7575
(rule (simplify (bxor bigty (uextend _ x@(value_type smallty)) (uextend _ y@(value_type smallty))))
7676
(uextend bigty (bxor smallty x y)))
7777

78-
;; Matches values where `ireducing` them will not actually introduce another
79-
;; instruction, since other rules will collapse them with the reduction.
80-
(decl pure multi will_simplify_with_ireduce_rec (u8 Value) Value)
81-
(rule (will_simplify_with_ireduce_rec _ x@(uextend _ _)) x)
82-
(rule (will_simplify_with_ireduce_rec _ x@(sextend _ _)) x)
83-
(rule (will_simplify_with_ireduce_rec _ x@(iconst _ _)) x)
84-
(rule (will_simplify_with_ireduce_rec depth x@(unary_op _ _ a))
85-
(if-let _ (u8_lt 0 depth))
86-
(if-let _ (reducible_modular_op x))
87-
(if-let _ (will_simplify_with_ireduce_rec (u8_sub depth 1) a))
88-
x)
89-
(rule (will_simplify_with_ireduce_rec depth x@(binary_op _ _ a b))
90-
(if-let _ (u8_lt 0 depth))
91-
(if-let _ (reducible_modular_op x))
92-
(if-let _ (will_simplify_with_ireduce_rec (u8_sub depth 1) a))
93-
(if-let _ (will_simplify_with_ireduce_rec (u8_sub depth 1) b))
94-
x)
95-
96-
(decl pure multi will_simplify_with_ireduce (Value) Value)
97-
(rule (will_simplify_with_ireduce x)
98-
(will_simplify_with_ireduce_rec 2 x))
99-
78+
;; Replace `(small)(x OP y)` with `(small)x OP (small)y` in cases where that's
79+
;; legal.
10080
;; Matches values where the high bits of the input don't affect lower bits of
10181
;; the output, and thus the inputs can be reduced before the operation rather
10282
;; than doing the wide operation then reducing afterwards.
103-
(decl pure multi reducible_modular_op (Value) Value)
104-
(rule (reducible_modular_op x@(ineg _ _)) x)
105-
(rule (reducible_modular_op x@(bnot _ _)) x)
106-
(rule (reducible_modular_op x@(iadd _ _ _)) x)
107-
(rule (reducible_modular_op x@(isub _ _ _)) x)
108-
(rule (reducible_modular_op x@(imul _ _ _)) x)
109-
(rule (reducible_modular_op x@(bor _ _ _)) x)
110-
(rule (reducible_modular_op x@(bxor _ _ _)) x)
111-
(rule (reducible_modular_op x@(band _ _ _)) x)
83+
(rule (simplify (ireduce ty (ineg _ x))) (ineg ty (ireduce ty x)))
84+
(rule (simplify (ireduce ty (bnot _ x))) (bnot ty (ireduce ty x)))
11285

113-
;; Replace `(small)(x OP y)` with `(small)x OP (small)y` in cases where that's
114-
;; legal and it reduces the total number of instructions since the reductions
115-
;; to the arguments simplify further.
116-
(rule (simplify (ireduce smallty val@(unary_op _ op x)))
117-
(if-let _ (reducible_modular_op val))
118-
(if-let _ (will_simplify_with_ireduce x))
119-
(unary_op smallty op (ireduce smallty x)))
120-
(rule (simplify (ireduce smallty val@(binary_op _ op x y)))
121-
(if-let _ (reducible_modular_op val))
122-
(if-let _ (will_simplify_with_ireduce x))
123-
(if-let _ (will_simplify_with_ireduce y))
124-
(binary_op smallty op (ireduce smallty x) (ireduce smallty y)))
86+
(rule (simplify (ireduce ty (iadd _ x y))) (iadd ty (ireduce ty x) (ireduce ty y)))
87+
(rule (simplify (ireduce ty (isub _ x y))) (isub ty (ireduce ty x) (ireduce ty y)))
88+
(rule (simplify (ireduce ty (imul _ x y))) (imul ty (ireduce ty x) (ireduce ty y)))
89+
(rule (simplify (ireduce ty (bor _ x y))) (bor ty (ireduce ty x) (ireduce ty y)))
90+
(rule (simplify (ireduce ty (bxor _ x y))) (bxor ty (ireduce ty x) (ireduce ty y)))
91+
(rule (simplify (ireduce ty (band _ x y))) (band ty (ireduce ty x) (ireduce ty y)))

cranelift/codegen/src/prelude_opt.isle

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,3 @@
120120
(extractor (sextend_maybe ty val) (sextend_maybe_etor ty val))
121121
(rule 0 (sextend_maybe ty val) (sextend ty val))
122122
(rule 1 (sextend_maybe ty val@(value_type ty)) val)
123-
124-
(decl unary_op (Type Opcode Value) Value)
125-
(extractor (unary_op ty opcode x)
126-
(inst_data ty (InstructionData.Unary opcode x)))
127-
(rule (unary_op ty opcode x)
128-
(make_inst ty (InstructionData.Unary opcode x)))
129-
130-
(decl binary_op (Type Opcode Value Value) Value)
131-
(extractor (binary_op ty opcode x y)
132-
(inst_data ty (InstructionData.Binary opcode (value_array_2 x y))))
133-
(rule (binary_op ty opcode x y)
134-
(make_inst ty (InstructionData.Binary opcode (value_array_2_ctor x y))))

0 commit comments

Comments
 (0)