From ea919489ee86d2b00e2e91bb318700d70fdd02f6 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Thu, 5 Sep 2019 18:33:13 +0530 Subject: [PATCH] [codegen] add encodings for iadd carry variants (#961) * [codegen] add encodings for iadd carry variants Add encodings for iadd carry variants (iadd_cout, iadd_cin, iadd_carry) for x86_32, enabling the legalization for iadd.i64 to work. * [codegen] remove support for iadd carry variants on riscv Previously, the carry variants of iadd (iadd_cin, iadd_cout and iadd_carry) were being legalized for isa/riscv since RISC architectures lack a flags register. This forced us to return and accept booleans for these operations, which proved to be problematic and inconvenient, especially for x86. This commit removes support for said statements and all dependent statements for isa/riscv so that we can work on a better legalization strategy in the future. * [codegen] change operand type from bool to iflag for iadd carry variants The type of the carry operands for the carry variants of the iadd instruction (iadd_cin, iadd_cout, iadd_carry) was bool for compatibility reasons for isa/riscv. Since support for these instructions on RISC architectures has been temporarily suspended, we can safely change the type to iflags. --- .../codegen/meta/src/isa/x86/encodings.rs | 9 ++++ cranelift/codegen/meta/src/isa/x86/recipes.rs | 41 +++++++++++++++++++ .../codegen/meta/src/shared/instructions.rs | 4 +- cranelift/codegen/meta/src/shared/legalize.rs | 28 ------------- cranelift/codegen/src/ir/dfg.rs | 5 +-- .../{expand-i32.clif => expand-i32.clif.bak} | 3 ++ ...egalize-i64.clif => legalize-i64.clif.bak} | 3 ++ .../filetests/filetests/isa/x86/binary32.clif | 8 ++++ .../filetests/isa/x86/legalize-i64.clif | 16 ++++++++ 9 files changed, 84 insertions(+), 33 deletions(-) rename cranelift/filetests/filetests/isa/riscv/{expand-i32.clif => expand-i32.clif.bak} (87%) rename cranelift/filetests/filetests/isa/riscv/{legalize-i64.clif => legalize-i64.clif.bak} (94%) create mode 100644 cranelift/filetests/filetests/isa/x86/legalize-i64.clif diff --git a/cranelift/codegen/meta/src/isa/x86/encodings.rs b/cranelift/codegen/meta/src/isa/x86/encodings.rs index 02186983b3..ab929011b7 100644 --- a/cranelift/codegen/meta/src/isa/x86/encodings.rs +++ b/cranelift/codegen/meta/src/isa/x86/encodings.rs @@ -372,6 +372,9 @@ pub fn define( let fsub = shared.by_name("fsub"); let func_addr = shared.by_name("func_addr"); let iadd = shared.by_name("iadd"); + let iadd_cout = shared.by_name("iadd_cout"); + let iadd_cin = shared.by_name("iadd_cin"); + let iadd_carry = shared.by_name("iadd_carry"); let iadd_imm = shared.by_name("iadd_imm"); let icmp = shared.by_name("icmp"); let icmp_imm = shared.by_name("icmp_imm"); @@ -556,6 +559,8 @@ pub fn define( let rec_rfurm = r.template("rfurm"); let rec_rmov = r.template("rmov"); let rec_rr = r.template("rr"); + let rec_rcin = r.template("rcin"); + let rec_rcarry = r.template("rcarry"); let rec_rrx = r.template("rrx"); let rec_safepoint = r.recipe("safepoint"); let rec_setf_abcd = r.template("setf_abcd"); @@ -611,6 +616,10 @@ pub fn define( let mut e = PerCpuModeEncodings::new(); e.enc_i32_i64(iadd, rec_rr.opcodes(vec![0x01])); + e.enc_i32_i64(iadd_cout, rec_rr.opcodes(vec![0x01])); + e.enc_i32_i64(iadd_cin, rec_rcin.opcodes(vec![0x11])); + e.enc_i32_i64(iadd_carry, rec_rcarry.opcodes(vec![0x11])); + e.enc_i32_i64(isub, rec_rr.opcodes(vec![0x29])); e.enc_i32_i64(band, rec_rr.opcodes(vec![0x21])); e.enc_i32_i64(bor, rec_rr.opcodes(vec![0x09])); diff --git a/cranelift/codegen/meta/src/isa/x86/recipes.rs b/cranelift/codegen/meta/src/isa/x86/recipes.rs index d126580804..6da640a172 100644 --- a/cranelift/codegen/meta/src/isa/x86/recipes.rs +++ b/cranelift/codegen/meta/src/isa/x86/recipes.rs @@ -2528,6 +2528,47 @@ pub fn define<'shared>( ), ); + // Adding with carry + + // XX /r, MR form. Add two GPR registers and get carry flag. + recipes.add_template_recipe( + EncodingRecipeBuilder::new("rcin", f_ternary, 1) + .operands_in(vec![ + OperandConstraint::RegClass(gpr), + OperandConstraint::RegClass(gpr), + OperandConstraint::FixedReg(reg_rflags), + ]) + .operands_out(vec![0]) + .clobbers_flags(true) + .emit( + r#" + {{PUT_OP}}(bits, rex2(in_reg0, in_reg1), sink); + modrm_rr(in_reg0, in_reg1, sink); + "#, + ), + ); + + // XX /r, MR form. Add two GPR registers with carry flag. + recipes.add_template_recipe( + EncodingRecipeBuilder::new("rcarry", f_ternary, 1) + .operands_in(vec![ + OperandConstraint::RegClass(gpr), + OperandConstraint::RegClass(gpr), + OperandConstraint::FixedReg(reg_rflags), + ]) + .operands_out(vec![ + OperandConstraint::TiedInput(0), + OperandConstraint::FixedReg(reg_rflags), + ]) + .clobbers_flags(true) + .emit( + r#" + {{PUT_OP}}(bits, rex2(in_reg0, in_reg1), sink); + modrm_rr(in_reg0, in_reg1, sink); + "#, + ), + ); + // Compare and set flags. // XX /r, MR form. Compare two GPR registers and set flags. diff --git a/cranelift/codegen/meta/src/shared/instructions.rs b/cranelift/codegen/meta/src/shared/instructions.rs index a40a6dc247..02819be42b 100644 --- a/cranelift/codegen/meta/src/shared/instructions.rs +++ b/cranelift/codegen/meta/src/shared/instructions.rs @@ -1864,8 +1864,8 @@ pub fn define( let a = &operand("a", iB); let x = &operand("x", iB); let y = &operand("y", iB); - let c_in = &operand_doc("c_in", b1, "Input carry flag"); - let c_out = &operand_doc("c_out", b1, "Output carry flag"); + let c_in = &operand_doc("c_in", iflags, "Input carry flag"); + let c_out = &operand_doc("c_out", iflags, "Output carry flag"); let b_in = &operand_doc("b_in", b1, "Input borrow flag"); let b_out = &operand_doc("b_out", b1, "Output borrow flag"); diff --git a/cranelift/codegen/meta/src/shared/legalize.rs b/cranelift/codegen/meta/src/shared/legalize.rs index 004bdff95b..05c74fdc47 100644 --- a/cranelift/codegen/meta/src/shared/legalize.rs +++ b/cranelift/codegen/meta/src/shared/legalize.rs @@ -66,7 +66,6 @@ pub fn define(insts: &InstructionGroup, immediates: &OperandKinds) -> TransformG let fcvt_from_uint = insts.by_name("fcvt_from_uint"); let fneg = insts.by_name("fneg"); let iadd = insts.by_name("iadd"); - let iadd_carry = insts.by_name("iadd_carry"); let iadd_cin = insts.by_name("iadd_cin"); let iadd_cout = insts.by_name("iadd_cout"); let iadd_imm = insts.by_name("iadd_imm"); @@ -168,8 +167,6 @@ pub fn define(insts: &InstructionGroup, immediates: &OperandKinds) -> TransformG let c2 = var("c2"); let c3 = var("c3"); let c4 = var("c4"); - let c_in = var("c_in"); - let c_int = var("c_int"); let d = var("d"); let d1 = var("d1"); let d2 = var("d2"); @@ -464,27 +461,12 @@ pub fn define(insts: &InstructionGroup, immediates: &OperandKinds) -> TransformG // Expand integer operations with carry for RISC architectures that don't have // the flags. - let intcc_ult = Literal::enumerator_for(intcc, "ult"); - expand.legalize( - def!((a, c) = iadd_cout(x, y)), - vec![def!(a = iadd(x, y)), def!(c = icmp(intcc_ult, a, x))], - ); - let intcc_ugt = Literal::enumerator_for(intcc, "ugt"); expand.legalize( def!((a, b) = isub_bout(x, y)), vec![def!(a = isub(x, y)), def!(b = icmp(intcc_ugt, a, x))], ); - expand.legalize( - def!(a = iadd_cin(x, y, c)), - vec![ - def!(a1 = iadd(x, y)), - def!(c_int = bint(c)), - def!(a = iadd(a1, c_int)), - ], - ); - expand.legalize( def!(a = isub_bin(x, y, b)), vec![ @@ -494,16 +476,6 @@ pub fn define(insts: &InstructionGroup, immediates: &OperandKinds) -> TransformG ], ); - expand.legalize( - def!((a, c) = iadd_carry(x, y, c_in)), - vec![ - def!((a1, c1) = iadd_cout(x, y)), - def!(c_int = bint(c_in)), - def!((a, c2) = iadd_cout(a1, c_int)), - def!(c = bor(c1, c2)), - ], - ); - expand.legalize( def!((a, b) = isub_borrow(x, y, b_in)), vec![ diff --git a/cranelift/codegen/src/ir/dfg.rs b/cranelift/codegen/src/ir/dfg.rs index 7392e893e7..24eeff4eec 100644 --- a/cranelift/codegen/src/ir/dfg.rs +++ b/cranelift/codegen/src/ir/dfg.rs @@ -1239,7 +1239,6 @@ mod tests { #[test] fn aliases() { - use crate::ir::condcodes::IntCC; use crate::ir::InstBuilder; let mut func = Function::new(); @@ -1264,9 +1263,9 @@ mod tests { pos.func.dfg.clear_results(iadd); pos.func.dfg.attach_result(iadd, s); - // Replace `iadd_cout` with a normal `iadd` and an `icmp`. + // Replace `iadd_cout` with a normal `iadd` and an `ifcmp`. pos.func.dfg.replace(iadd).iadd(v1, arg0); - let c2 = pos.ins().icmp(IntCC::UnsignedLessThan, s, v1); + let c2 = pos.ins().ifcmp(s, v1); pos.func.dfg.change_to_alias(c, c2); assert_eq!(pos.func.dfg.resolve_aliases(c2), c2); diff --git a/cranelift/filetests/filetests/isa/riscv/expand-i32.clif b/cranelift/filetests/filetests/isa/riscv/expand-i32.clif.bak similarity index 87% rename from cranelift/filetests/filetests/isa/riscv/expand-i32.clif rename to cranelift/filetests/filetests/isa/riscv/expand-i32.clif.bak index eb63d7cdcd..cc2a9d726c 100644 --- a/cranelift/filetests/filetests/isa/riscv/expand-i32.clif +++ b/cranelift/filetests/filetests/isa/riscv/expand-i32.clif.bak @@ -1,3 +1,6 @@ +; TODO(ryzokuken): figure out a better legalization strategy for platforms that +; platforms that don't have flags. + ; Test the legalization of i32 instructions that don't have RISC-V versions. test legalizer diff --git a/cranelift/filetests/filetests/isa/riscv/legalize-i64.clif b/cranelift/filetests/filetests/isa/riscv/legalize-i64.clif.bak similarity index 94% rename from cranelift/filetests/filetests/isa/riscv/legalize-i64.clif rename to cranelift/filetests/filetests/isa/riscv/legalize-i64.clif.bak index d043337a21..366472e7d3 100644 --- a/cranelift/filetests/filetests/isa/riscv/legalize-i64.clif +++ b/cranelift/filetests/filetests/isa/riscv/legalize-i64.clif.bak @@ -1,3 +1,6 @@ +; TODO(ryzokuken): figure out a better legalization strategy for platforms that +; platforms that don't have flags. + ; Test the legalization of i64 arithmetic instructions. test legalizer target riscv32 supports_m=1 diff --git a/cranelift/filetests/filetests/isa/x86/binary32.clif b/cranelift/filetests/filetests/isa/x86/binary32.clif index 602b2d4f31..79680a939d 100644 --- a/cranelift/filetests/filetests/isa/x86/binary32.clif +++ b/cranelift/filetests/filetests/isa/x86/binary32.clif @@ -469,6 +469,14 @@ ebb0: ; asm: mov %cl,(%eax,%ebx,1) istore8_complex v601, v521+v522 ; bin: heap_oob 88 0c 18 + ; Carry Addition + ; asm: addl %esi, %ecx + [-,%rcx,%rflags] v701, v702 = iadd_cout v1, v2 ; bin: 01 f1 + ; asm: adcl %esi, %ecx + [-,%rcx] v703 = iadd_cin v1, v2, v702 ; bin: 11 f1 + ; asm: adcl %esi, %ecx + [-,%rcx,%rflags] v704, v705 = iadd_carry v1, v2, v702 ; bin: 11 f1 + ; asm: testl %ecx, %ecx ; asm: je ebb1 brz v1, ebb1 ; bin: 85 c9 74 0e diff --git a/cranelift/filetests/filetests/isa/x86/legalize-i64.clif b/cranelift/filetests/filetests/isa/x86/legalize-i64.clif new file mode 100644 index 0000000000..dc7d5696fe --- /dev/null +++ b/cranelift/filetests/filetests/isa/x86/legalize-i64.clif @@ -0,0 +1,16 @@ +; Test the legalization of i64 instructions on x86_32. +test legalizer +target i686 haswell + +; regex: V=v\d+ + +function %iadd(i64, i64) -> i64 { +ebb0(v1: i64, v2: i64): + v10 = iadd v1, v2 + ; check: v1 = iconcat $(v1_lsb=$V), $(v1_msb=$V) + ; check: v2 = iconcat $(v2_lsb=$V), $(v2_msb=$V) + ; check: $(v10_lsb=$V), $(carry=$V) = iadd_cout $v1_lsb, $v2_lsb + ; check: $(v10_msb=$V) = iadd_cin $v1_msb, $v2_msb, $carry + ; check: v10 = iconcat $v10_lsb, $v10_msb + return v10 +}