Skip to content

Commit 2b261cd

Browse files
committed
egraphs: Delete select+fcmp to f{min,max}_pseudo transform
This transform is wrong for the inputs in the attached testcase. It was introduced in bytecodealliance#5546 and has been sort of unnoticed until bytecodealliance#6843 caused some wasmtime testcode to fire this optimization rule and produce this counterexample.
1 parent 11af6af commit 2b261cd

File tree

3 files changed

+16
-42
lines changed

3 files changed

+16
-42
lines changed

cranelift/codegen/src/opts/selects.isle

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,3 @@
4444
(rule (simplify (bitselect ty @ (multi_lane _ _) (ugt _ x y) y x)) (umin ty x y))
4545
(rule (simplify (bitselect ty @ (multi_lane _ _) (uge _ x y) y x)) (umin ty x y))
4646

47-
;; For floats convert fcmp lt into pseudo_min and gt into pseudo_max
48-
;;
49-
;; fmax_pseudo docs state:
50-
;; The behaviour for this operations is defined as fmax_pseudo(a, b) = (a < b) ? b : a, and the behaviour for zero
51-
;; or NaN inputs follows from the behaviour of < with such inputs.
52-
;;
53-
;; That is exactly the operation that we match here!
54-
(rule (simplify
55-
(select ty (fcmp _ (FloatCC.LessThan) x y) x y))
56-
(fmin_pseudo ty x y))
57-
(rule (simplify
58-
(select ty (fcmp _ (FloatCC.GreaterThan) x y) x y))
59-
(fmax_pseudo ty x y))
60-
61-
;; TODO: perform this same optimization to `f{min,max}_pseudo` for vectors
62-
;; with the `bitselect` instruction, but the pattern is a bit more complicated
63-
;; due to most bitselects-over-floats having bitcasts.

cranelift/filetests/filetests/egraph/select.clif

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -141,28 +141,3 @@ block0(v0: i32, v1: i32, v2: i32, v3: i32):
141141
; check: v4 = icmp ule v0, v1
142142
; check: v5 = select v4, v2, v3
143143
; check: return v5
144-
145-
146-
147-
148-
function %select_fcmp_gt_to_fmax_pseudo(f32, f32) -> f32 {
149-
block0(v0: f32, v1: f32):
150-
v2 = fcmp gt v0, v1
151-
v3 = select v2, v0, v1
152-
return v3
153-
}
154-
155-
; check: block0(v0: f32, v1: f32):
156-
; check: v4 = fmax_pseudo v0, v1
157-
; check: return v4
158-
159-
function %select_fcmp_lt_to_fmin_pseudo(f32, f32) -> f32 {
160-
block0(v0: f32, v1: f32):
161-
v2 = fcmp lt v0, v1
162-
v3 = select v2, v0, v1
163-
return v3
164-
}
165-
166-
; check: block0(v0: f32, v1: f32):
167-
; check: v4 = fmin_pseudo v0, v1
168-
; check: return v4
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
test interpret
2+
test run
3+
set opt_level=speed
4+
target x86_64
5+
target x86_64 has_avx
6+
target aarch64
7+
target s390x
8+
target riscv64
9+
10+
function %a(f32, f32) -> f32 fast {
11+
block0(v0: f32, v1: f32):
12+
v2 = fcmp lt v0, v1
13+
v3 = select v2, v0, v1
14+
return v3
15+
}
16+
; run: %a(0.0, NaN) == NaN

0 commit comments

Comments
 (0)