T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

x64: fix miscompilation of select.i128 (#4017)

Browse Source 
Issue #3963 identified a miscompilation with select in which the second
in the pair of `CMOV`s (one pair per `i128` register) used the wrong
flag. This change fixes the error in the x64 ISLE helper function
emitting these `CMOV` instructions.
This commit is contained in:
Andrew Brown
2022-04-12 09:56:57 -07:00
committed by GitHub
parent 54aa720506
commit 7a55779c6b
5 changed files with 42 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cranelift/codegen/src/isa/x64/inst.isle
View File

@@ -1780,9 +1780,9 @@
(tmp2 WritableGpr (temp_writable_gpr))
(size OperandSize (OperandSize.Size64))
(cmove1 MInst (MInst.Cmove size cc1 (value_regs_get_gpr cons 0) (value_regs_get_gpr alt 0) tmp1))
(cmove2 MInst (MInst.Cmove size cc1 (value_regs_get_gpr cons 0) tmp1 dst1))
(cmove2 MInst (MInst.Cmove size cc2 (value_regs_get_gpr cons 0) tmp1 dst1))
(cmove3 MInst (MInst.Cmove size cc1 (value_regs_get_gpr cons 1) (value_regs_get_gpr alt 1) tmp2))
(cmove4 MInst (MInst.Cmove size cc1 (value_regs_get_gpr cons 1) tmp2 dst2)))
(cmove4 MInst (MInst.Cmove size cc2 (value_regs_get_gpr cons 1) tmp2 dst2)))
(ConsumesFlags.ConsumesFlagsFourTimesReturnsValueRegs
cmove1
cmove2

2
cranelift/codegen/src/isa/x64/lower/isle/generated_code.manifest
View File

@@ -1,4 +1,4 @@
src/clif.isle 443b34b797fc8ace
src/prelude.isle c0751050a11e2686
src/isa/x64/inst.isle c4729db7808ba0b5
src/isa/x64/inst.isle 1a4206dba9fcf9d8
src/isa/x64/lower.isle 7e839e6b667bfe77

4
cranelift/codegen/src/isa/x64/lower/isle/generated_code.rs generated
View File

@@ -2636,7 +2636,7 @@ pub fn constructor_cmove_or_from_values<C: Context>(
let expr16_0 = C::writable_gpr_to_gpr(ctx, expr4_0);
let expr17_0 = MInst::Cmove {
size: expr6_0,
cc: pattern2_0.clone(),
cc: pattern3_0.clone(),
consequent: expr15_0,
alternative: expr16_0,
dst: expr2_0,
@@ -2659,7 +2659,7 @@ pub fn constructor_cmove_or_from_values<C: Context>(
let expr27_0 = C::writable_gpr_to_gpr(ctx, expr5_0);
let expr28_0 = MInst::Cmove {
size: expr6_0,
cc: pattern2_0.clone(),
cc: pattern3_0.clone(),
consequent: expr26_0,
alternative: expr27_0,
dst: expr3_0,

27
cranelift/filetests/filetests/isa/x64/select-i128.clif
View File

@@ -28,3 +28,30 @@ block0(v0: i32, v1: i128, v2: i128):
; Inst 10: ret
; }}
;; Test for issue: https://github.com/bytecodealliance/wasmtime/issues/3963.
function %f1(f32, i128, i128) -> i128 {
block0(v0: f32, v1: i128, v2: i128):
v3 = fcmp.f32 eq v0, v0
v4 = select.i128 v3, v1, v2
return v4
}
; VCode_ShowWithRRU {{
; Entry block: 0
; Block 0:
; (original IR block: block0)
; (instruction range: 0 .. 12)
; Inst 0: pushq %rbp
; Inst 1: movq %rsp, %rbp
; Inst 2: ucomiss %xmm0, %xmm0
; Inst 3: cmovnzq %rdx, %rdi
; Inst 4: cmovpq %rdx, %rdi
; Inst 5: cmovnzq %rcx, %rsi
; Inst 6: cmovpq %rcx, %rsi
; Inst 7: movq %rdi, %rax
; Inst 8: movq %rsi, %rdx
; Inst 9: movq %rbp, %rsp
; Inst 10: popq %rbp
; Inst 11: ret
; }}

11
cranelift/filetests/filetests/runtests/i128-select.clif
View File

@@ -10,6 +10,15 @@ block0(v0: b1, v1: i128, v2: i128):
}
; run: %i128_select(true, 0, 1) == 0
; run: %i128_select(false, 0, 1) == 1
; run: %i128_select(true, 0x00000000_00000000_DECAFFFF_C0FFEEEE, 0xFFFFFFFF_FFFFFFFF_C0FFEEEE_DECAFFFF) == 0x00000000_00000000_DECAFFFF_C0FFEEEE
; run: %i128_select(false, 0x00000000_00000000_DECAFFFF_C0FFEEEE, 0xFFFFFFFF_FFFFFFFF_C0FFEEEE_DECAFFFF) == 0xFFFFFFFF_FFFFFFFF_C0FFEEEE_DECAFFFF
;; Test for issue: https://github.com/bytecodealliance/wasmtime/issues/3963.
function %i128_fcmp_eq_select(f32, i128, i128) -> i128 {
block0(v0: f32, v1: i128, v2: i128):
v3 = fcmp eq v0, v0
v4 = select.i128 v3, v1, v2
return v4
}
; run: %i128_fcmp_eq_select(0x42.42, 1, 0) == 1
; run: %i128_fcmp_eq_select(NaN, 1, 0) == 0