Skip to content

Commit

Permalink
pulley: Fix mistakes in compare-with-immediate (bytecodealliance#9870)
Browse files Browse the repository at this point in the history
This commit fixes a few mistakes that were introduced in bytecodealliance#9863.
Specifically when lowering `Cond.If...` and the arguments needed
swapping the condition was also inverted by accident. More `*.clif`
runtests were added to catch this case and expose it. Additionally
Pulley now has lowering for all the `FloatCC` orderings to be able to
run the `select.clif` runtest which primarily exposed the issue.
  • Loading branch information
alexcrichton authored Dec 20, 2024
1 parent a179f95 commit 68976ba
Show file tree
Hide file tree
Showing 24 changed files with 652 additions and 10 deletions.
30 changes: 20 additions & 10 deletions cranelift/codegen/src/isa/pulley_shared/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,13 @@
(rule (lower_fcmp $F32 (FloatCC.LessThanOrEqual) a b) (pulley_flteq32 a b))
(rule (lower_fcmp $F64 (FloatCC.LessThanOrEqual) a b) (pulley_flteq64 a b))

;; NB: Pulley doesn't have lowerings for `Ordered` or `Unordered` `FloatCC`
;; conditions as that's not needed by wasm at this time.
;; Ordered == !a.is_nan() && !b.is_nan()
(rule (lower_fcmp ty (FloatCC.Ordered) a b)
(pulley_xband32 (lower_fcmp ty (FloatCC.Equal) a a) (lower_fcmp ty (FloatCC.Equal) b b)))

;; OrderedNotEqual == a < b || a > b
(rule (lower_fcmp ty (FloatCC.OrderedNotEqual) a b)
(pulley_xbor32 (lower_fcmp ty (FloatCC.LessThan) a b) (lower_fcmp ty (FloatCC.GreaterThan) a b)))

;; Pulley doesn't have instructions for `>` and `>=`, so we have to reverse the
;; operation.
Expand All @@ -737,6 +742,11 @@
(rule (lower_fcmp ty (FloatCC.GreaterThanOrEqual) a b)
(lower_fcmp ty (FloatCC.LessThanOrEqual) b a))

;; For other `Unordered*` comparisons generate its complement and invert the result.
(rule -1 (lower_fcmp ty cc a b)
(if-let true (floatcc_unordered cc))
(pulley_xbxor32_s8 (lower_fcmp ty (floatcc_complement cc) a b) 1))

;;;; Rules for `load` and friends ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(decl amode (Value Offset32) Amode)
Expand Down Expand Up @@ -937,13 +947,13 @@

;; Note the operand swaps here
(rule (emit_cond (Cond.IfXsgt32I32 src1 src2))
(pulley_xslteq32 (imm $I32 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXsgteq32I32 src1 src2))
(pulley_xslt32 (imm $I32 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXsgteq32I32 src1 src2))
(pulley_xslteq32 (imm $I32 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXugt32I32 src1 src2))
(pulley_xulteq32 (imm $I32 (u32_as_u64 src2)) src1))
(rule (emit_cond (Cond.IfXugteq32I32 src1 src2))
(pulley_xult32 (imm $I32 (u32_as_u64 src2)) src1))
(rule (emit_cond (Cond.IfXugteq32I32 src1 src2))
(pulley_xulteq32 (imm $I32 (u32_as_u64 src2)) src1))

(rule (emit_cond (Cond.IfXeq64I32 src1 src2))
(pulley_xeq64 src1 (imm $I64 (i64_as_u64 (i32_as_i64 src2)))))
Expand All @@ -960,13 +970,13 @@

;; Note the operand swaps here
(rule (emit_cond (Cond.IfXsgt64I32 src1 src2))
(pulley_xslteq64 (imm $I64 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXsgteq64I32 src1 src2))
(pulley_xslt64 (imm $I64 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXsgteq64I32 src1 src2))
(pulley_xslteq64 (imm $I64 (i64_as_u64 (i32_as_i64 src2))) src1))
(rule (emit_cond (Cond.IfXugt64I32 src1 src2))
(pulley_xulteq64 (imm $I64 (u32_as_u64 src2)) src1))
(rule (emit_cond (Cond.IfXugteq64I32 src1 src2))
(pulley_xult64 (imm $I64 (u32_as_u64 src2)) src1))
(rule (emit_cond (Cond.IfXugteq64I32 src1 src2))
(pulley_xulteq64 (imm $I64 (u32_as_u64 src2)) src1))

;;;; Rules for `bitcast` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ge.clif
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ target aarch64
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ge_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-one.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_one_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ord.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ord_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ueq.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ueq_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-uge.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_uge_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ugt.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ugt_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ule.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ule_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-ult.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fcmp_ult_f32(f32, f32) -> i8 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fcmp-uno.clif
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ target x86_64 has_avx
target s390x
target riscv64
target riscv64 has_c has_zcb
target pulley32
target pulley32be
target pulley64
target pulley64be


function %fcmp_uno_f32(f32, f32) -> i8 {
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fmax-pseudo.clif
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ target riscv64
target riscv64 has_zfa
target riscv64 has_c has_zcb
target s390x
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fmax_p_f32(f32, f32) -> f32 {
block0(v0: f32, v1: f32):
Expand Down
4 changes: 4 additions & 0 deletions cranelift/filetests/filetests/runtests/fmin-pseudo.clif
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ target riscv64
target riscv64 has_zfa
target riscv64 has_c has_zcb
target s390x
target pulley32
target pulley32be
target pulley64
target pulley64be

function %fmin_p_f32(f32, f32) -> f32 {
block0(v0: f32, v1: f32):
Expand Down
36 changes: 36 additions & 0 deletions cranelift/filetests/filetests/runtests/icmp-eq.clif
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,39 @@ block0(v0: i64, v1: i64):
; run: %icmp_eq_i64(0, 0) == 1
; run: %icmp_eq_i64(1, 0) == 0
; run: %icmp_eq_i64(-1, -1) == 1

function %icmp_eq_i8_imm(i8) -> i8 {
block0(v0: i8):
v2 = icmp_imm eq v0, 10
return v2
}
; run: %icmp_eq_i8_imm(10) == 1
; run: %icmp_eq_i8_imm(0) == 0
; run: %icmp_eq_i8_imm(-1) == 0

function %icmp_eq_i16_imm(i16) -> i8 {
block0(v0: i16):
v2 = icmp_imm eq v0, 10
return v2
}
; run: %icmp_eq_i16_imm(10) == 1
; run: %icmp_eq_i16_imm(0) == 0
; run: %icmp_eq_i16_imm(-1) == 0

function %icmp_eq_i32_imm(i32) -> i8 {
block0(v0: i32):
v2 = icmp_imm eq v0, 10
return v2
}
; run: %icmp_eq_i32_imm(10) == 1
; run: %icmp_eq_i32_imm(0) == 0
; run: %icmp_eq_i32_imm(-1) == 0

function %icmp_eq_i64_imm(i64) -> i8 {
block0(v0: i64):
v2 = icmp_imm eq v0, 10
return v2
}
; run: %icmp_eq_i64_imm(10) == 1
; run: %icmp_eq_i64_imm(0) == 0
; run: %icmp_eq_i64_imm(-1) == 0
36 changes: 36 additions & 0 deletions cranelift/filetests/filetests/runtests/icmp-ne.clif
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,39 @@ block0(v0: i64, v1: i64):
; run: %icmp_ne_i64(0, 0) == 0
; run: %icmp_ne_i64(1, 0) == 1
; run: %icmp_ne_i64(-1, -1) == 0

function %icmp_ne_i8_imm(i8) -> i8 {
block0(v0: i8):
v2 = icmp_imm ne v0, 10
return v2
}
; run: %icmp_ne_i8_imm(10) == 0
; run: %icmp_ne_i8_imm(0) == 1
; run: %icmp_ne_i8_imm(-1) == 1

function %icmp_ne_i16_imm(i16) -> i8 {
block0(v0: i16):
v2 = icmp_imm ne v0, 10
return v2
}
; run: %icmp_ne_i16_imm(10) == 0
; run: %icmp_ne_i16_imm(0) == 1
; run: %icmp_ne_i16_imm(-1) == 1

function %icmp_ne_i32_imm(i32) -> i8 {
block0(v0: i32):
v2 = icmp_imm ne v0, 10
return v2
}
; run: %icmp_ne_i32_imm(10) == 0
; run: %icmp_ne_i32_imm(0) == 1
; run: %icmp_ne_i32_imm(-1) == 1

function %icmp_ne_i64_imm(i64) -> i8 {
block0(v0: i64):
v2 = icmp_imm ne v0, 10
return v2
}
; run: %icmp_ne_i64_imm(10) == 0
; run: %icmp_ne_i64_imm(0) == 1
; run: %icmp_ne_i64_imm(-1) == 1
36 changes: 36 additions & 0 deletions cranelift/filetests/filetests/runtests/icmp-sge.clif
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,39 @@ block0(v0: i64, v1: i64):
; run: %icmp_sge_i64(0, 1) == 0
; run: %icmp_sge_i64(-5, -1) == 0
; run: %icmp_sge_i64(1, -1) == 1

function %icmp_sge_i8_imm(i8) -> i8 {
block0(v0: i8):
v2 = icmp_imm sge v0, 10
return v2
}
; run: %icmp_sge_i8_imm(10) == 1
; run: %icmp_sge_i8_imm(0) == 0
; run: %icmp_sge_i8_imm(-1) == 0

function %icmp_sge_i16_imm(i16) -> i8 {
block0(v0: i16):
v2 = icmp_imm sge v0, 10
return v2
}
; run: %icmp_sge_i16_imm(10) == 1
; run: %icmp_sge_i16_imm(0) == 0
; run: %icmp_sge_i16_imm(-1) == 0

function %icmp_sge_i32_imm(i32) -> i8 {
block0(v0: i32):
v2 = icmp_imm sge v0, 10
return v2
}
; run: %icmp_sge_i32_imm(10) == 1
; run: %icmp_sge_i32_imm(0) == 0
; run: %icmp_sge_i32_imm(-1) == 0

function %icmp_sge_i64_imm(i64) -> i8 {
block0(v0: i64):
v2 = icmp_imm sge v0, 10
return v2
}
; run: %icmp_sge_i64_imm(10) == 1
; run: %icmp_sge_i64_imm(0) == 0
; run: %icmp_sge_i64_imm(-1) == 0
36 changes: 36 additions & 0 deletions cranelift/filetests/filetests/runtests/icmp-sgt.clif
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,39 @@ block0(v0: i64, v1: i64):
; run: %icmp_sgt_i64(0, 1) == 0
; run: %icmp_sgt_i64(-5, -1) == 0
; run: %icmp_sgt_i64(1, -1) == 1

function %icmp_sgt_i8_imm(i8) -> i8 {
block0(v0: i8):
v2 = icmp_imm sgt v0, 10
return v2
}
; run: %icmp_sgt_i8_imm(10) == 0
; run: %icmp_sgt_i8_imm(0) == 0
; run: %icmp_sgt_i8_imm(-1) == 0

function %icmp_sgt_i16_imm(i16) -> i8 {
block0(v0: i16):
v2 = icmp_imm sgt v0, 10
return v2
}
; run: %icmp_sgt_i16_imm(10) == 0
; run: %icmp_sgt_i16_imm(0) == 0
; run: %icmp_sgt_i16_imm(-1) == 0

function %icmp_sgt_i32_imm(i32) -> i8 {
block0(v0: i32):
v2 = icmp_imm sgt v0, 10
return v2
}
; run: %icmp_sgt_i32_imm(10) == 0
; run: %icmp_sgt_i32_imm(0) == 0
; run: %icmp_sgt_i32_imm(-1) == 0

function %icmp_sgt_i64_imm(i64) -> i8 {
block0(v0: i64):
v2 = icmp_imm sgt v0, 10
return v2
}
; run: %icmp_sgt_i64_imm(10) == 0
; run: %icmp_sgt_i64_imm(0) == 0
; run: %icmp_sgt_i64_imm(-1) == 0
36 changes: 36 additions & 0 deletions cranelift/filetests/filetests/runtests/icmp-sle.clif
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,39 @@ block0(v0: i64, v1: i64):
; run: %icmp_sle_i64(0, 1) == 1
; run: %icmp_sle_i64(-5, -1) == 1
; run: %icmp_sle_i64(1, -1) == 0

function %icmp_sle_i8_imm(i8) -> i8 {
block0(v0: i8):
v2 = icmp_imm sle v0, 10
return v2
}
; run: %icmp_sle_i8_imm(10) == 1
; run: %icmp_sle_i8_imm(0) == 1
; run: %icmp_sle_i8_imm(-1) == 1

function %icmp_sle_i16_imm(i16) -> i8 {
block0(v0: i16):
v2 = icmp_imm sle v0, 10
return v2
}
; run: %icmp_sle_i16_imm(10) == 1
; run: %icmp_sle_i16_imm(0) == 1
; run: %icmp_sle_i16_imm(-1) == 1

function %icmp_sle_i32_imm(i32) -> i8 {
block0(v0: i32):
v2 = icmp_imm sle v0, 10
return v2
}
; run: %icmp_sle_i32_imm(10) == 1
; run: %icmp_sle_i32_imm(0) == 1
; run: %icmp_sle_i32_imm(-1) == 1

function %icmp_sle_i64_imm(i64) -> i8 {
block0(v0: i64):
v2 = icmp_imm sle v0, 10
return v2
}
; run: %icmp_sle_i64_imm(10) == 1
; run: %icmp_sle_i64_imm(0) == 1
; run: %icmp_sle_i64_imm(-1) == 1
Loading

0 comments on commit 68976ba

Please sign in to comment.