From 8cde5b7e8b00c934ff97affbe11e591dcce9c610 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Wed, 9 Mar 2022 13:25:08 -0800 Subject: [PATCH] Cherrypick and release-notes update for 0.35.1. (#3909) * Fix uextend on x64 for non-i32-source cases. (#3906) 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! * Updated RELEASES for 0.35.1 patch release. --- RELEASES.md | 11 + cranelift/codegen/src/isa/x64/lower.isle | 20 +- .../x64/lower/isle/generated_code.manifest | 2 +- .../src/isa/x64/lower/isle/generated_code.rs | 186 +++++++-------- .../filetests/filetests/runtests/extend.clif | 216 ++++++++++++++++-- 5 files changed, 321 insertions(+), 114 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 1b0522be1aab..9b24a63ee9e8 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -2,6 +2,17 @@ -------------------------------------------------------------------------------- +## 0.35.1 + +Released 2022-03-09. + +### Fixed + +* Fixed a bug in the x86-64 lowering of the `uextend` opcode for narrow (`i8`, + `i16`) integer sources when the value is produced by one of several + arithmetic instructions. + [#3906](https://github.com/bytecodealliance/wasmtime/pull/3906) + ## 0.35.0 Released 2022-03-07. diff --git a/cranelift/codegen/src/isa/x64/lower.isle b/cranelift/codegen/src/isa/x64/lower.isle index d082fda8ef5e..cb7636939d6f 100644 --- a/cranelift/codegen/src/isa/x64/lower.isle +++ b/cranelift/codegen/src/isa/x64/lower.isle @@ -1894,34 +1894,34 @@ ;; keep these here than write an external extractor containing bits of ;; the instruction pattern.s) (rule (lower (has_type $I64 - (uextend src @ (iadd _ _)))) + (uextend src @ (has_type $I32 (iadd _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (iadd_ifcout _ _)))) + (uextend src @ (has_type $I32 (iadd_ifcout _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (isub _ _)))) + (uextend src @ (has_type $I32 (isub _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (imul _ _)))) + (uextend src @ (has_type $I32 (imul _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (band _ _)))) + (uextend src @ (has_type $I32 (band _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (bor _ _)))) + (uextend src @ (has_type $I32 (bor _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (bxor _ _)))) + (uextend src @ (has_type $I32 (bxor _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (ishl _ _)))) + (uextend src @ (has_type $I32 (ishl _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (ushr _ _)))) + (uextend src @ (has_type $I32 (ushr _ _))))) src) (rule (lower (has_type $I64 - (uextend src @ (uload32 _ _ _)))) + (uextend src @ (has_type $I32 (uload32 _ _ _))))) src) ;; Rules for `sextend` / `bextend` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/codegen/src/isa/x64/lower/isle/generated_code.manifest b/cranelift/codegen/src/isa/x64/lower/isle/generated_code.manifest index b1f03e50d893..ebb1176c915f 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle/generated_code.manifest +++ b/cranelift/codegen/src/isa/x64/lower/isle/generated_code.manifest @@ -1,4 +1,4 @@ src/clif.isle 9ea75a6f790b5c03 src/prelude.isle b2bc986bcbbbb77 src/isa/x64/inst.isle 40f495d3ca5ae547 -src/isa/x64/lower.isle faa2a07bba48a813 +src/isa/x64/lower.isle c049f7d36db0e0fb diff --git a/cranelift/codegen/src/isa/x64/lower/isle/generated_code.rs b/cranelift/codegen/src/isa/x64/lower/isle/generated_code.rs index b409fad5c4b3..ab52c9cadd61 100644 --- a/cranelift/codegen/src/isa/x64/lower/isle/generated_code.rs +++ b/cranelift/codegen/src/isa/x64/lower/isle/generated_code.rs @@ -4665,101 +4665,107 @@ pub fn constructor_lower(ctx: &mut C, arg0: Inst) -> Option { if let Some(pattern7_0) = C::def_inst(ctx, pattern5_1) { - let pattern8_0 = C::inst_data(ctx, pattern7_0); - match &pattern8_0 { - &InstructionData::Binary { - opcode: ref pattern9_0, - args: ref pattern9_1, - } => { - match pattern9_0 { - &Opcode::Iadd => { - let (pattern11_0, pattern11_1) = - C::unpack_value_array_2(ctx, pattern9_1); - // 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 (pattern11_0, pattern11_1) = - C::unpack_value_array_2(ctx, pattern9_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 (pattern11_0, pattern11_1) = - C::unpack_value_array_2(ctx, pattern9_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 (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); + if let Some(pattern8_0) = C::first_result(ctx, pattern7_0) { + let pattern9_0 = C::value_type(ctx, pattern8_0); + if pattern9_0 == I32 { + let pattern11_0 = C::inst_data(ctx, pattern7_0); + match &pattern11_0 { + &InstructionData::Binary { + opcode: ref pattern12_0, + args: ref pattern12_1, + } => { + match pattern12_0 { + &Opcode::Iadd => { + let (pattern14_0, pattern14_1) = + C::unpack_value_array_2(ctx, pattern12_1); + // 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::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 pattern12_0, + arg: pattern12_1, + flags: pattern12_2, + offset: pattern12_3, + } => { + if let &Opcode::Uload32 = pattern12_0 { + // Rule at src/isa/x64/lower.isle line 1923. + 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); diff --git a/cranelift/filetests/filetests/runtests/extend.clif b/cranelift/filetests/filetests/runtests/extend.clif index 524177de1054..42e77f7f0bb6 100644 --- a/cranelift/filetests/filetests/runtests/extend.clif +++ b/cranelift/filetests/filetests/runtests/extend.clif @@ -1,23 +1,213 @@ test run target aarch64 -target arm target s390x target x86_64 -function %uextend() -> b1 { -block0: - v0 = iconst.i32 0xffff_ee00 +;;;; basic uextend + +function %uextend8_16(i8) -> i16 { +block0(v0: i8): + v1 = uextend.i16 v0 + return v1 +} +; 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 - v2 = icmp_imm eq v1, 0xffff_ee00 - return v2 + return v1 } -; run +; 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 %sextend() -> b1 { -block0: - v0 = iconst.i32 0xffff_ee00 +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 - v2 = icmp_imm eq v1, 0xffff_ffff_ffff_ee00 - return v2 + return v1 } -; run +; 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 +