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

Fix uextend on x64 for non-i32-source cases. (#3906)

Browse Source 
In #3849, I moved uextend over to ISLE in the x64 backend. Unfortunately, the lowering patterns had a bug in the i32-to-i64 special case (when we know the generating instruction zeroes the upper 32 bits): it wasn't actually special casing for an i32 source! This meant that e.g. zero extends of the results of i8 adds did not work properly.

This PR fixes the bug and updates the runtest for extends significantly to cover the narrow-value cases.

No security impact to Wasm as Wasm does not use narrow integer types.

Thanks @bjorn3 for reporting!
This commit is contained in:
Chris Fallin
2022-03-09 11:10:59 -08:00
committed by GitHub
parent f21aa98ccb
commit 26ce9a3853
4 changed files with 313 additions and 117 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
cranelift/codegen/src/isa/x64/lower.isle
View File

@@ -1894,34 +1894,34 @@
;; keep these here than write an external extractor containing bits of ;; keep these here than write an external extractor containing bits of
;; the instruction pattern.s) ;; the instruction pattern.s)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (iadd _ _)))) (uextend src @ (has_type $I32 (iadd _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (iadd_ifcout _ _)))) (uextend src @ (has_type $I32 (iadd_ifcout _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (isub _ _)))) (uextend src @ (has_type $I32 (isub _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (imul _ _)))) (uextend src @ (has_type $I32 (imul _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (band _ _)))) (uextend src @ (has_type $I32 (band _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (bor _ _)))) (uextend src @ (has_type $I32 (bor _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (bxor _ _)))) (uextend src @ (has_type $I32 (bxor _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (ishl _ _)))) (uextend src @ (has_type $I32 (ishl _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (ushr _ _)))) (uextend src @ (has_type $I32 (ushr _ _)))))
src) src)
(rule (lower (has_type $I64 (rule (lower (has_type $I64
(uextend src @ (uload32 _ _ _)))) (uextend src @ (has_type $I32 (uload32 _ _ _)))))
src) src)
;; Rules for `sextend` / `bextend` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Rules for `sextend` / `bextend` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

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

@@ -1,4 +1,4 @@
src/clif.isle 9ea75a6f790b5c03 src/clif.isle 9ea75a6f790b5c03
src/prelude.isle b2bc986bcbbbb77 src/prelude.isle b2bc986bcbbbb77
src/isa/x64/inst.isle 40f495d3ca5ae547 src/isa/x64/inst.isle 40f495d3ca5ae547
src/isa/x64/lower.isle faa2a07bba48a813 src/isa/x64/lower.isle c049f7d36db0e0fb

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

@@ -4665,101 +4665,107 @@ pub fn constructor_lower<C: Context>(ctx: &mut C, arg0: Inst) -> Option<InstOutp
} }
&Opcode::Uextend => { &Opcode::Uextend => {
if let Some(pattern7_0) = C::def_inst(ctx, pattern5_1) { if let Some(pattern7_0) = C::def_inst(ctx, pattern5_1) {
let pattern8_0 = C::inst_data(ctx, pattern7_0); if let Some(pattern8_0) = C::first_result(ctx, pattern7_0) {
match &pattern8_0 { let pattern9_0 = C::value_type(ctx, pattern8_0);
&InstructionData::Binary { if pattern9_0 == I32 {
opcode: ref pattern9_0, let pattern11_0 = C::inst_data(ctx, pattern7_0);
args: ref pattern9_1, match &pattern11_0 {
} => { &InstructionData::Binary {
match pattern9_0 { opcode: ref pattern12_0,
&Opcode::Iadd => { args: ref pattern12_1,
let (pattern11_0, pattern11_1) = } => {
C::unpack_value_array_2(ctx, pattern9_1); match pattern12_0 {
// Rule at src/isa/x64/lower.isle line 1896. &Opcode::Iadd => {
let expr0_0 = let (pattern14_0, pattern14_1) =
constructor_output_value(ctx, pattern5_1)?; C::unpack_value_array_2(ctx, pattern12_1);
return Some(expr0_0); // Rule at src/isa/x64/lower.isle line 1896.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Isub => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1902.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Imul => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1905.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::IaddIfcout => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1899.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Band => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1908.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Bor => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1911.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Bxor => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1914.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Ishl => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1917.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Ushr => {
let (pattern14_0, pattern14_1) =
C::unpack_value_array_2(ctx, pattern12_1);
// Rule at src/isa/x64/lower.isle line 1920.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
_ => {}
}
} }
&Opcode::Isub => { &InstructionData::Load {
let (pattern11_0, pattern11_1) = opcode: ref pattern12_0,
C::unpack_value_array_2(ctx, pattern9_1); arg: pattern12_1,
// Rule at src/isa/x64/lower.isle line 1902. flags: pattern12_2,
let expr0_0 = offset: pattern12_3,
constructor_output_value(ctx, pattern5_1)?; } => {
return Some(expr0_0); if let &Opcode::Uload32 = pattern12_0 {
} // Rule at src/isa/x64/lower.isle line 1923.
&Opcode::Imul => { let expr0_0 =
let (pattern11_0, pattern11_1) = constructor_output_value(ctx, pattern5_1)?;
C::unpack_value_array_2(ctx, pattern9_1); return Some(expr0_0);
// Rule at src/isa/x64/lower.isle line 1905. }
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::IaddIfcout => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1899.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Band => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1908.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Bor => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1911.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Bxor => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1914.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Ishl => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1917.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
&Opcode::Ushr => {
let (pattern11_0, pattern11_1) =
C::unpack_value_array_2(ctx, pattern9_1);
// Rule at src/isa/x64/lower.isle line 1920.
let expr0_0 =
constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
} }
_ => {} _ => {}
} }
} }
&InstructionData::Load {
opcode: ref pattern9_0,
arg: pattern9_1,
flags: pattern9_2,
offset: pattern9_3,
} => {
if let &Opcode::Uload32 = pattern9_0 {
// Rule at src/isa/x64/lower.isle line 1923.
let expr0_0 = constructor_output_value(ctx, pattern5_1)?;
return Some(expr0_0);
}
}
_ => {}
} }
} }
let pattern7_0 = C::value_type(ctx, pattern5_1); let pattern7_0 = C::value_type(ctx, pattern5_1);

222
cranelift/filetests/filetests/runtests/extend.clif
View File

@@ -1,23 +1,213 @@
test run test run
target aarch64 target aarch64
target arm
target s390x target s390x
target x86_64 target x86_64
function %uextend() -> b1 { ;;;; basic uextend
block0:
v0 = iconst.i32 0xffff_ee00
v1 = uextend.i64 v0
v2 = icmp_imm eq v1, 0xffff_ee00
return v2
}
; run
function %sextend() -> b1 { function %uextend8_16(i8) -> i16 {
block0: block0(v0: i8):
v0 = iconst.i32 0xffff_ee00 v1 = uextend.i16 v0
v1 = sextend.i64 v0 return v1
v2 = icmp_imm eq v1, 0xffff_ffff_ffff_ee00
return v2
} }
; run ; run: %uextend8_16(0xfe) == 0xfe
; run: %uextend8_16(0x7e) == 0x7e
function %uextend8_32(i8) -> i32 {
block0(v0: i8):
v1 = uextend.i32 v0
return v1
}
; run: %uextend8_32(0xfe) == 0xfe
; run: %uextend8_32(0x7e) == 0x7e
function %uextend16_32(i16) -> i32 {
block0(v0: i16):
v1 = uextend.i32 v0
return v1
}
; run: %uextend16_32(0xfe00) == 0xfe00
; run: %uextend16_32(0x7e00) == 0x7e00
function %uextend8_64(i8) -> i64 {
block0(v0: i8):
v1 = uextend.i64 v0
return v1
}
; run: %uextend8_64(0xfe) == 0xfe
; run: %uextend8_64(0x7e) == 0x7e
function %uextend16_64(i16) -> i64 {
block0(v0: i16):
v1 = uextend.i64 v0
return v1
}
; run: %uextend16_64(0xfe00) == 0xfe00
; run: %uextend16_64(0x7e00) == 0x7e00
function %uextend32_64(i32) -> i64 {
block0(v0: i32):
v1 = uextend.i64 v0
return v1
}
; run: %uextend32_64(0xffff_ee00) == 0xffff_ee00
; run: %uextend32_64(0x7fff_ee00) == 0x7fff_ee00
;;;; basic sextend
function %sextend8_16(i8) -> i16 {
block0(v0: i8):
v1 = sextend.i16 v0
return v1
}
; run: %sextend8_16(0xff) == 0xffff
; run: %sextend8_16(0x7f) == 0x7f
function %sextend8_32(i8) -> i32 {
block0(v0: i8):
v1 = sextend.i32 v0
return v1
}
; run: %sextend8_32(0xff) == 0xffff_ffff
; run: %sextend8_32(0x7f) == 0x7f
function %sextend16_32(i16) -> i32 {
block0(v0: i16):
v1 = sextend.i32 v0
return v1
}
; run: %sextend16_32(0xfe00) == 0xffff_fe00
; run: %sextend16_32(0x7e00) == 0x7e00
function %sextend8_64(i8) -> i64 {
block0(v0: i8):
v1 = sextend.i64 v0
return v1
}
; run: %sextend8_64(0xff) == 0xffff_ffff_ffff_ffff
; run: %sextend8_64(0x7f) == 0x7f
function %sextend16_64(i16) -> i64 {
block0(v0: i16):
v1 = sextend.i64 v0
return v1
}
; run: %sextend16_64(0xfe00) == 0xffff_ffff_ffff_fe00
; run: %sextend16_64(0x7e00) == 0x7e00
function %sextend32_64(i32) -> i64 {
block0(v0: i32):
v1 = sextend.i64 v0
return v1
}
; run: %sextend32_64(0xffff_ee00) == 0xffff_ffff_ffff_ee00
; run: %sextend32_64(0x7fff_ee00) == 0x7fff_ee00
;; uextend of an `add` that we know is likely to set undefined bits
;; above the narrow value
function %add_uextend8_16(i8, i8) -> i16 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = uextend.i16 v2
return v3
}
; run: %add_uextend8_16(0xfe, 0x03) == 0x0001
function %add_uextend8_32(i8, i8) -> i32 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = uextend.i32 v2
return v3
}
; run: %add_uextend8_32(0xfe, 0x03) == 0x0000_0001
function %add_uextend16_32(i16, i16) -> i32 {
block0(v0: i16, v1: i16):
v2 = iadd.i16 v0, v1
v3 = uextend.i32 v2
return v3
}
; run: %add_uextend16_32(0xfe00, 0x302) == 0x0000_0102
function %add_uextend8_64(i8, i8) -> i64 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = uextend.i64 v2
return v3
}
; run: %add_uextend8_64(0xfe, 0x03) == 0x0000_0000_0000_0001
function %add_uextend16_64(i16, i16) -> i64 {
block0(v0: i16, v1: i16):
v2 = iadd.i16 v0, v1
v3 = uextend.i64 v2
return v3
}
; run: %add_uextend16_64(0xfe00, 0x302) == 0x0000_0000_0000_0102
function %add_uextend32_64(i32, i32) -> i64 {
block0(v0: i32, v1: i32):
v2 = iadd.i32 v0, v1
v3 = uextend.i64 v2
return v3
}
; run: %add_uextend32_64(0xffff_ee00, 0x1000_0001) == 0x0000_0000_0fff_ee01
;; sextend of an `add` that we know is likely to set undefined bits
;; above the narrow value
function %add_sextend8_16(i8, i8) -> i16 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = sextend.i16 v2
return v3
}
; run: %add_sextend8_16(0xfe, 0x03) == 0x0001
; run: %add_sextend8_16(0xfe, 0x83) == 0xff81
function %add_sextend8_32(i8, i8) -> i32 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = sextend.i32 v2
return v3
}
; run: %add_sextend8_32(0xfe, 0x03) == 0x0000_0001
; run: %add_sextend8_32(0xfe, 0x83) == 0xffff_ff81
function %add_sextend16_32(i16, i16) -> i32 {
block0(v0: i16, v1: i16):
v2 = iadd.i16 v0, v1
v3 = sextend.i32 v2
return v3
}
; run: %add_sextend16_32(0xfe00, 0x302) == 0x0000_0102
; run: %add_sextend16_32(0xfe00, 0x8302) == 0xffff_8102
function %add_sextend8_64(i8, i8) -> i64 {
block0(v0: i8, v1: i8):
v2 = iadd.i8 v0, v1
v3 = sextend.i64 v2
return v3
}
; run: %add_sextend8_64(0xfe, 0x03) == 0x0000_0000_0000_0001
; run: %add_sextend8_64(0xfe, 0x83) == 0xffff_ffff_ffff_ff81
function %add_sextend16_64(i16, i16) -> i64 {
block0(v0: i16, v1: i16):
v2 = iadd.i16 v0, v1
v3 = sextend.i64 v2
return v3
}
; run: %add_sextend16_64(0xfe00, 0x302) == 0x0000_0000_0000_0102
; run: %add_sextend16_64(0xfe00, 0x8302) == 0xffff_ffff_ffff_8102
function %add_sextend32_64(i32, i32) -> i64 {
block0(v0: i32, v1: i32):
v2 = iadd.i32 v0, v1
v3 = sextend.i64 v2
return v3
}
; run: %add_sextend32_64(0xffff_ee00, 0x1000_0001) == 0x0000_0000_0fff_ee01
; run: %add_sextend32_64(0xffff_ee00, 0x9000_0001) == 0xffff_ffff_8fff_ee01