From a27a079d65e1867f3b39475ea4e40392f568e84c Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Wed, 27 May 2020 09:24:46 -0700 Subject: [PATCH] Replace ExtractLane format with BinaryImm8 Like https://github.com/bytecodealliance/wasmtime/pull/1762, this change the name of the `ExtractLane` format to the more-general `BinaryImm8` and renames its immediate argument from `lane` to `imm`. --- .../codegen/meta/src/isa/x86/instructions.rs | 4 ++-- cranelift/codegen/meta/src/isa/x86/recipes.rs | 16 +++++++-------- cranelift/codegen/meta/src/shared/formats.rs | 9 +++------ .../codegen/meta/src/shared/instructions.rs | 2 +- cranelift/codegen/src/isa/x86/enc_tables.rs | 4 ++-- cranelift/codegen/src/verifier/mod.rs | 6 +++--- cranelift/codegen/src/write.rs | 2 +- cranelift/reader/src/parser.rs | 12 +++++------ cranelift/serde/src/serde_clif_json.rs | 20 +++++++++---------- 9 files changed, 36 insertions(+), 39 deletions(-) diff --git a/cranelift/codegen/meta/src/isa/x86/instructions.rs b/cranelift/codegen/meta/src/isa/x86/instructions.rs index b15be24546..f516a928dd 100644 --- a/cranelift/codegen/meta/src/isa/x86/instructions.rs +++ b/cranelift/codegen/meta/src/isa/x86/instructions.rs @@ -283,7 +283,7 @@ pub(crate) fn define( Packed Shuffle Doublewords -- copies data from either memory or lanes in an extended register and re-orders the data according to the passed immediate byte. "#, - &formats.extract_lane, + &formats.binary_imm8, ) .operands_in(vec![a, i]) // TODO allow copying from memory here (need more permissive type than TxN) .operands_out(vec![a]), @@ -314,7 +314,7 @@ pub(crate) fn define( The lane index, ``Idx``, is an immediate value, not an SSA value. It must indicate a valid lane index for the type of ``x``. "#, - &formats.extract_lane, + &formats.binary_imm8, ) .operands_in(vec![x, Idx]) .operands_out(vec![a]), diff --git a/cranelift/codegen/meta/src/isa/x86/recipes.rs b/cranelift/codegen/meta/src/isa/x86/recipes.rs index db28f73e03..690cdf84d0 100644 --- a/cranelift/codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift/codegen/meta/src/isa/x86/recipes.rs @@ -996,20 +996,20 @@ pub(crate) fn define<'shared>( // XX /r ib with 8-bit unsigned immediate (e.g. for pshufd) { recipes.add_template_inferred( - EncodingRecipeBuilder::new("r_ib_unsigned_fpr", &formats.extract_lane, 2) + EncodingRecipeBuilder::new("r_ib_unsigned_fpr", &formats.binary_imm8, 2) .operands_in(vec![fpr]) .operands_out(vec![fpr]) .inst_predicate(InstructionPredicate::new_is_unsigned_int( - &*formats.extract_lane, - "lane", + &*formats.binary_imm8, + "imm", 8, 0, - )) // TODO if the format name is changed then "lane" should be renamed to something more appropriate--ordering mask? broadcast immediate? + )) .emit( r#" {{PUT_OP}}(bits, rex2(in_reg0, out_reg0), sink); modrm_rr(in_reg0, out_reg0, sink); - let imm:i64 = lane.into(); + let imm: i64 = imm.into(); sink.put1(imm as u8); "#, ), @@ -1020,17 +1020,17 @@ pub(crate) fn define<'shared>( // XX /r ib with 8-bit unsigned immediate (e.g. for extractlane) { recipes.add_template_inferred( - EncodingRecipeBuilder::new("r_ib_unsigned_gpr", &formats.extract_lane, 2) + EncodingRecipeBuilder::new("r_ib_unsigned_gpr", &formats.binary_imm8, 2) .operands_in(vec![fpr]) .operands_out(vec![gpr]) .inst_predicate(InstructionPredicate::new_is_unsigned_int( - &*formats.extract_lane, "lane", 8, 0, + &*formats.binary_imm8, "imm", 8, 0, )) .emit( r#" {{PUT_OP}}(bits, rex2(out_reg0, in_reg0), sink); modrm_rr(out_reg0, in_reg0, sink); // note the flipped register in the ModR/M byte - let imm:i64 = lane.into(); + let imm: i64 = imm.into(); sink.put1(imm as u8); "#, ), "size_with_inferred_rex_for_inreg0_outreg0" diff --git a/cranelift/codegen/meta/src/shared/formats.rs b/cranelift/codegen/meta/src/shared/formats.rs index 455920eba5..10d4937524 100644 --- a/cranelift/codegen/meta/src/shared/formats.rs +++ b/cranelift/codegen/meta/src/shared/formats.rs @@ -17,7 +17,7 @@ pub(crate) struct Formats { pub(crate) cond_trap: Rc, pub(crate) copy_special: Rc, pub(crate) copy_to_ssa: Rc, - pub(crate) extract_lane: Rc, + pub(crate) binary_imm8: Rc, pub(crate) float_compare: Rc, pub(crate) float_cond: Rc, pub(crate) float_cond_trap: Rc, @@ -76,6 +76,8 @@ impl Formats { binary: Builder::new("Binary").value().value().build(), + binary_imm8: Builder::new("BinaryImm8").value().imm(&imm.uimm8).build(), + binary_imm: Builder::new("BinaryImm").value().imm(&imm.imm64).build(), // The select instructions are controlled by the second VALUE operand. @@ -100,11 +102,6 @@ impl Formats { nullary: Builder::new("NullAry").build(), - extract_lane: Builder::new("ExtractLane") - .value() - .imm_with_name("lane", &imm.uimm8) - .build(), - shuffle: Builder::new("Shuffle") .value() .value() diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index f2bed0e330..7ba624fa46 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -579,7 +579,7 @@ fn define_simd_lane_access( may or may not be zeroed depending on the ISA but the type system should prevent using ``a`` as anything other than the extracted value. "#, - &formats.extract_lane, + &formats.binary_imm8, ) .operands_in(vec![x, Idx]) .operands_out(vec![a]), diff --git a/cranelift/codegen/src/isa/x86/enc_tables.rs b/cranelift/codegen/src/isa/x86/enc_tables.rs index a2897c6d78..19b08d58a2 100644 --- a/cranelift/codegen/src/isa/x86/enc_tables.rs +++ b/cranelift/codegen/src/isa/x86/enc_tables.rs @@ -1195,10 +1195,10 @@ fn convert_extractlane( let mut pos = FuncCursor::new(func).at_inst(inst); pos.use_srcloc(inst); - if let ir::InstructionData::ExtractLane { + if let ir::InstructionData::BinaryImm8 { opcode: ir::Opcode::Extractlane, arg, - lane, + imm: lane, } = pos.func.dfg[inst] { // NOTE: the following legalization assumes that the upper bits of the XMM register do diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 5010446309..029a437b4f 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -756,10 +756,10 @@ impl<'a> Verifier<'a> { | UnaryIeee64 { .. } | UnaryBool { .. } | Binary { .. } + | BinaryImm8 { .. } | BinaryImm { .. } | Ternary { .. } | TernaryImm8 { .. } - | ExtractLane { .. } | Shuffle { .. } | IntCompare { .. } | IntCompareImm { .. } @@ -1912,9 +1912,9 @@ impl<'a> Verifier<'a> { Ok(()) } } - ir::InstructionData::ExtractLane { + ir::InstructionData::BinaryImm8 { opcode: ir::instructions::Opcode::Extractlane, - lane, + imm: lane, arg, .. } diff --git a/cranelift/codegen/src/write.rs b/cranelift/codegen/src/write.rs index 0aada7d79d..19287855ce 100644 --- a/cranelift/codegen/src/write.rs +++ b/cranelift/codegen/src/write.rs @@ -508,6 +508,7 @@ pub fn write_operands( constant_handle, .. } => write!(w, " {}", constant_handle), Binary { args, .. } => write!(w, " {}, {}", args[0], args[1]), + BinaryImm8 { arg, imm, .. } => write!(w, " {}, {}", arg, imm), BinaryImm { arg, imm, .. } => write!(w, " {}, {}", arg, imm), Ternary { args, .. } => write!(w, " {}, {}, {}", args[0], args[1], args[2]), MultiAry { ref args, .. } => { @@ -519,7 +520,6 @@ pub fn write_operands( } NullAry { .. } => write!(w, " "), TernaryImm8 { imm, args, .. } => write!(w, " {}, {}, {}", args[0], args[1], imm), - ExtractLane { lane, arg, .. } => write!(w, " {}, {}", arg, lane), Shuffle { mask, args, .. } => { let data = dfg.immediates.get(mask).expect( "Expected the shuffle mask to already be inserted into the immediates table", diff --git a/cranelift/reader/src/parser.rs b/cranelift/reader/src/parser.rs index 47e6e695fe..5bb83d5006 100644 --- a/cranelift/reader/src/parser.rs +++ b/cranelift/reader/src/parser.rs @@ -2752,6 +2752,12 @@ impl<'a> Parser<'a> { args: [lhs, rhs], } } + InstructionFormat::BinaryImm8 => { + let arg = self.match_value("expected SSA value first operand")?; + self.match_token(Token::Comma, "expected ',' between operands")?; + let imm = self.match_uimm8("expected unsigned 8-bit immediate")?; + InstructionData::BinaryImm8 { opcode, arg, imm } + } InstructionFormat::BinaryImm => { let lhs = self.match_value("expected SSA value first operand")?; self.match_token(Token::Comma, "expected ',' between operands")?; @@ -2899,12 +2905,6 @@ impl<'a> Parser<'a> { args: [lhs, rhs], } } - InstructionFormat::ExtractLane => { - let arg = self.match_value("expected SSA value last operand")?; - self.match_token(Token::Comma, "expected ',' between operands")?; - let lane = self.match_uimm8("expected lane number")?; - InstructionData::ExtractLane { opcode, lane, arg } - } InstructionFormat::Shuffle => { let a = self.match_value("expected SSA value first operand")?; self.match_token(Token::Comma, "expected ',' between operands")?; diff --git a/cranelift/serde/src/serde_clif_json.rs b/cranelift/serde/src/serde_clif_json.rs index efa0ee2815..b143a6fad0 100644 --- a/cranelift/serde/src/serde_clif_json.rs +++ b/cranelift/serde/src/serde_clif_json.rs @@ -32,6 +32,11 @@ pub enum SerInstData { opcode: String, args: [String; 2], }, + BinaryImm8 { + opcode: String, + arg: String, + imm: String, + }, BinaryImm { opcode: String, arg: String, @@ -53,11 +58,6 @@ pub enum SerInstData { NullAry { opcode: String, }, - ExtractLane { - opcode: String, - arg: String, - lane: String, - }, Shuffle { opcode: String, args: [String; 2], @@ -292,6 +292,11 @@ pub fn get_inst_data(inst_index: Inst, func: &Function) -> SerInstData { args: hold_args, } } + InstructionData::BinaryImm8 { opcode, arg, imm } => SerInstData::BinaryImm8 { + opcode: opcode.to_string(), + arg: arg.to_string(), + imm: imm.to_string(), + }, InstructionData::BinaryImm { opcode, arg, imm } => SerInstData::BinaryImm { opcode: opcode.to_string(), arg: arg.to_string(), @@ -331,11 +336,6 @@ pub fn get_inst_data(inst_index: Inst, func: &Function) -> SerInstData { imm: imm.to_string(), } } - InstructionData::ExtractLane { opcode, arg, lane } => SerInstData::ExtractLane { - opcode: opcode.to_string(), - arg: arg.to_string(), - lane: lane.to_string(), - }, InstructionData::UnaryConst { opcode, constant_handle,