Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x64: Begin migrating branch instructions to ISLE #4587

Merged
merged 6 commits into from
Aug 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions cranelift/codegen/src/isa/x64/inst.isle
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,16 @@

(type BoxVecMachLabel extern (enum))

(type MachLabelSlice extern (enum))

;; Extract a the target from a MachLabelSlice with exactly one target.
(decl single_target (MachLabel) MachLabelSlice)
(extern extractor single_target single_target)
Comment on lines +521 to +522
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a little doc comment for this? Something like

;; Match a label slice of length one, extracting the only label in the slice.


;; Extract a the targets from a MachLabelSlice with exactly two targets.
(decl two_targets (MachLabel MachLabel) MachLabelSlice)
(extern extractor two_targets two_targets)
Comment on lines +525 to +526
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar doc comment here please.


;; Get the `OperandSize` for a given `Type`, rounding smaller types up to 32 bits.
(decl operand_size_of_type_32_64 (Type) OperandSize)
(extern constructor operand_size_of_type_32_64 operand_size_of_type_32_64)
Expand Down Expand Up @@ -1057,9 +1067,13 @@
NLE
P
NP))

(decl intcc_to_cc (IntCC) CC)
(extern constructor intcc_to_cc intcc_to_cc)

(decl cc_invert (CC) CC)
(extern constructor cc_invert cc_invert)

(type AvxOpcode extern
(enum Vfmadd213ps
Vfmadd213pd))
Expand Down Expand Up @@ -3039,13 +3053,34 @@
(rule (trap_if_fcmp (FcmpCondResult.OrCondition producer cc1 cc2) tc)
(with_flags_side_effect producer (trap_if_or cc1 cc2 tc)))

;;;; Jumps ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; Unconditional jump.
(decl jmp_known (MachLabel) SideEffectNoResult)
(rule (jmp_known target)
(SideEffectNoResult.Inst (MInst.JmpKnown target)))

;; Conditional jump based on the condition code.
(decl jmp_cond (CC MachLabel MachLabel) ConsumesFlags)
(rule (jmp_cond cc taken not_taken)
(ConsumesFlags.ConsumesFlagsSideEffect (MInst.JmpCond cc taken not_taken)))

;; Conditional jump based on the result of an icmp.
(decl jmp_cond_icmp (IcmpCondResult MachLabel MachLabel) SideEffectNoResult)
(rule (jmp_cond_icmp (IcmpCondResult.Condition producer cc) taken not_taken)
(with_flags_side_effect producer (jmp_cond cc taken not_taken)))

;;;; Comparisons ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(type IcmpCondResult (enum (Condition (producer ProducesFlags) (cc CC))))

(decl icmp_cond_result (ProducesFlags CC) IcmpCondResult)
(rule (icmp_cond_result producer cc) (IcmpCondResult.Condition producer cc))

(decl invert_icmp_cond_result (IcmpCondResult) IcmpCondResult)
(rule (invert_icmp_cond_result (IcmpCondResult.Condition producer cc))
(icmp_cond_result producer (cc_invert cc)))

;; Lower an Icmp result into a boolean value in a register.
(decl lower_icmp_bool (IcmpCondResult) ValueRegs)
(rule (lower_icmp_bool (IcmpCondResult.Condition producer cc))
Expand Down
32 changes: 32 additions & 0 deletions cranelift/codegen/src/isa/x64/lower.isle
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
;; register(s) within which the lowered instruction's result values live.
(decl lower (Inst) InstOutput)

;; A variant of the main lowering constructor term, used for branches.
;; The only difference is that it gets an extra argument holding a vector
;; of branch targets to be used.
(decl lower_branch (Inst MachLabelSlice) InstOutput)

;;;; Rules for `iconst` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

;; `i64` and smaller.
Expand Down Expand Up @@ -2856,3 +2861,30 @@
(x64_load $I64
(Amode.ImmReg 8 (preg_rbp) (mem_flags_trusted))
(ExtKind.None)))

;; Rules for `jump` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower_branch (jump _ _) (single_target target))
(side_effect (jmp_known target)))

;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower_branch (brif cc (ifcmp a b) _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken)))

;; Rules for `brz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower_branch (brz (icmp cc a b) _ _) (two_targets taken not_taken))
(let ((cmp IcmpCondResult (invert_icmp_cond_result (emit_cmp cc a b))))
(side_effect (jmp_cond_icmp cmp taken not_taken))))

;; Rules for `brnz` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower_branch (brnz (icmp cc a b) _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken)))


;; Rules for `bricmp` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

(rule (lower_branch (br_icmp cc a b _ _) (two_targets taken not_taken))
(side_effect (jmp_cond_icmp (emit_cmp cc a b) taken not_taken)))
80 changes: 22 additions & 58 deletions cranelift/codegen/src/isa/x64/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2833,6 +2833,24 @@ impl LowerBackend for X64Backend {
// verifier pass.
assert!(branches.len() <= 2);

if let Ok(()) = isle::lower_branch(
ctx,
&self.triple,
&self.flags,
&self.x64_flags,
branches[0],
targets,
) {
return Ok(());
}

let implemented_in_isle = |ctx: &mut C| {
unreachable!(
"branch implemented in ISLE: inst = `{}`",
ctx.dfg().display_inst(branches[0])
)
};

if branches.len() == 2 {
// Must be a conditional branch followed by an unconditional branch.
let op0 = ctx.data(branches[0]).opcode();
Expand All @@ -2858,18 +2876,8 @@ impl LowerBackend for X64Backend {

let src_ty = ctx.input_ty(branches[0], 0);

if let Some(icmp) = matches_input(ctx, flag_input, Opcode::Icmp) {
let cond_code = ctx.data(icmp).cond_code().unwrap();
let cond_code = emit_cmp(ctx, icmp, cond_code);

let cond_code = if op0 == Opcode::Brz {
cond_code.inverse()
} else {
cond_code
};

let cc = CC::from_intcc(cond_code);
ctx.emit(Inst::jmp_cond(cc, taken, not_taken));
if let Some(_icmp) = matches_input(ctx, flag_input, Opcode::Icmp) {
implemented_in_isle(ctx)
} else if let Some(fcmp) = matches_input(ctx, flag_input, Opcode::Fcmp) {
let cond_code = ctx.data(fcmp).fp_cond_code().unwrap();
let cond_code = if op0 == Opcode::Brz {
Expand Down Expand Up @@ -2960,49 +2968,7 @@ impl LowerBackend for X64Backend {
}
}

Opcode::BrIcmp => {
let src_ty = ctx.input_ty(branches[0], 0);
if is_int_or_ref_ty(src_ty) || is_bool_ty(src_ty) {
let lhs = put_input_in_reg(
ctx,
InsnInput {
insn: branches[0],
input: 0,
},
);
let rhs = input_to_reg_mem_imm(
ctx,
InsnInput {
insn: branches[0],
input: 1,
},
);
let cc = CC::from_intcc(ctx.data(branches[0]).cond_code().unwrap());
// Cranelift's icmp semantics want to compare lhs - rhs, while Intel gives
// us dst - src at the machine instruction level, so invert operands.
ctx.emit(Inst::cmp_rmi_r(OperandSize::from_ty(src_ty), rhs, lhs));
ctx.emit(Inst::jmp_cond(cc, taken, not_taken));
} else {
unimplemented!("bricmp with non-int type {:?}", src_ty);
}
}

Opcode::Brif => {
let flag_input = InsnInput {
insn: branches[0],
input: 0,
};

if let Some(ifcmp) = matches_input(ctx, flag_input, Opcode::Ifcmp) {
let cond_code = ctx.data(branches[0]).cond_code().unwrap();
let cond_code = emit_cmp(ctx, ifcmp, cond_code);
let cc = CC::from_intcc(cond_code);
ctx.emit(Inst::jmp_cond(cc, taken, not_taken));
} else {
// Should be disallowed by flags checks in verifier.
unimplemented!("Brif with non-ifcmp input");
}
}
Opcode::BrIcmp | Opcode::Brif => implemented_in_isle(ctx),
Opcode::Brff => {
let flag_input = InsnInput {
insn: branches[0],
Expand Down Expand Up @@ -3039,9 +3005,7 @@ impl LowerBackend for X64Backend {
// Must be an unconditional branch or trap.
let op = ctx.data(branches[0]).opcode();
match op {
Opcode::Jump => {
ctx.emit(Inst::jmp_known(targets[0]));
}
Opcode::Jump => implemented_in_isle(ctx),

Opcode::BrTable => {
let jt_size = targets.len() - 1;
Expand Down
46 changes: 46 additions & 0 deletions cranelift/codegen/src/isa/x64/lower/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use target_lexicon::Triple;

type BoxCallInfo = Box<CallInfo>;
type BoxVecMachLabel = Box<SmallVec<[MachLabel; 4]>>;
type MachLabelSlice = [MachLabel];

pub struct SinkableLoad {
inst: Inst,
Expand Down Expand Up @@ -71,6 +72,28 @@ where
)
}

pub(crate) fn lower_branch<C>(
lower_ctx: &mut C,
triple: &Triple,
flags: &Flags,
isa_flags: &IsaFlags,
branch: Inst,
targets: &[MachLabel],
) -> Result<(), ()>
where
C: LowerCtx<I = MInst>,
{
lower_common(
lower_ctx,
triple,
flags,
isa_flags,
&[],
branch,
|cx, insn| generated_code::constructor_lower_branch(cx, insn, targets),
)
}

impl<C> Context for IsleContext<'_, C, Flags, IsaFlags, 6>
where
C: LowerCtx<I = MInst>,
Expand Down Expand Up @@ -562,6 +585,11 @@ where
CC::from_intcc(*intcc)
}

#[inline]
fn cc_invert(&mut self, cc: &CC) -> CC {
cc.invert()
}

#[inline]
fn sum_extend_fits_in_32_bits(
&mut self,
Expand Down Expand Up @@ -675,6 +703,24 @@ where

output_reg.to_reg()
}

#[inline]
fn single_target(&mut self, targets: &MachLabelSlice) -> Option<MachLabel> {
if targets.len() == 1 {
Some(targets[0])
} else {
None
}
}

#[inline]
fn two_targets(&mut self, targets: &MachLabelSlice) -> Option<(MachLabel, MachLabel)> {
if targets.len() == 2 {
Some((targets[0], targets[1]))
} else {
None
}
}
}

impl<C> IsleContext<'_, C, Flags, IsaFlags, 6>
Expand Down