From e8ecf1f809e46eef18e3eed2e018aabb9c4072b7 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Wed, 25 Oct 2017 11:14:11 -0700 Subject: [PATCH] Add a FixedTied constraint kind for operand constraints. Fixes #175. The Intel division instructions have fixed input operands that are clobbered by fixed output operands, so the value passed as an input will be clobbered just like a tied operand. The FixedTied operand constraint is used to indicate a fixed input operand that has a corresponding output operand with the same fixed register. Teach the spiller to teach a FixedTied operand the same as a Tied operand constraint and make sure that the input value is killed by the instruction. --- .../regalloc/output-interference.cton | 15 +++++++++++ lib/cretonne/meta/cdsl/isa.py | 10 +++++++ lib/cretonne/meta/cdsl/registers.py | 5 +++- lib/cretonne/meta/gen_encoding.py | 27 ++++++++++--------- lib/cretonne/src/isa/constraints.rs | 10 ++++++- lib/cretonne/src/regalloc/coloring.rs | 19 ++++++++++--- lib/cretonne/src/regalloc/spilling.rs | 4 +++ 7 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 cranelift/filetests/regalloc/output-interference.cton diff --git a/cranelift/filetests/regalloc/output-interference.cton b/cranelift/filetests/regalloc/output-interference.cton new file mode 100644 index 0000000000..11079a383f --- /dev/null +++ b/cranelift/filetests/regalloc/output-interference.cton @@ -0,0 +1,15 @@ +test compile +set is_64bit=1 +isa intel haswell + +function %test(i64) -> i64 native { +ebb0(v0: i64): + v2 = iconst.i64 12 + ; This division clobbers two of its fixed input registers on Intel. + ; These are FixedTied constraints that the spiller needs to resolve. + v5 = udiv v0, v2 + v6 = iconst.i64 13 + v9 = udiv v0, v6 + v10 = iadd v5, v9 + return v10 +} diff --git a/lib/cretonne/meta/cdsl/isa.py b/lib/cretonne/meta/cdsl/isa.py index 6d06f8ef0b..9fcfa440a3 100644 --- a/lib/cretonne/meta/cdsl/isa.py +++ b/lib/cretonne/meta/cdsl/isa.py @@ -394,6 +394,16 @@ class EncRecipe(object): o2i[o] = i return (i2o, o2i) + def fixed_ops(self): + # type: () -> Tuple[Set[Register], Set[Register]] + """ + Return two sets of registers representing the fixed input and output + operands. + """ + i = set(r for r in self.ins if isinstance(r, Register)) + o = set(r for r in self.outs if isinstance(r, Register)) + return (i, o) + def recipe_pred(self): # type: () -> RecipePred """ diff --git a/lib/cretonne/meta/cdsl/registers.py b/lib/cretonne/meta/cdsl/registers.py index 37d75d3d58..22e954ee8a 100644 --- a/lib/cretonne/meta/cdsl/registers.py +++ b/lib/cretonne/meta/cdsl/registers.py @@ -286,7 +286,10 @@ class RegClass(object): For example: `GPR.r5`. """ - return Register(self, self.bank.unit_by_name(attr)) + reg = Register(self, self.bank.unit_by_name(attr)) + # Save this register so we won't have to create it again. + setattr(self, attr, reg) + return reg def mask(self): # type: () -> List[int] diff --git a/lib/cretonne/meta/gen_encoding.py b/lib/cretonne/meta/gen_encoding.py index 43b00d37f6..283b9d1e0d 100644 --- a/lib/cretonne/meta/gen_encoding.py +++ b/lib/cretonne/meta/gen_encoding.py @@ -754,17 +754,14 @@ def emit_recipe_constraints(isa, fmt): for r in isa.all_recipes: fmt.comment(r.name) tied_i2o, tied_o2i = r.ties() + fixed_ins, fixed_outs = r.fixed_ops() with fmt.indented('RecipeConstraints {', '},'): - emit_operand_constraints(r, r.ins, 'ins', tied_i2o, fmt) - emit_operand_constraints(r, r.outs, 'outs', tied_o2i, fmt) - fmt.format( - 'fixed_ins: {},', - str(any(isinstance(c, Register) - for c in r.ins)).lower()) - fmt.format( - 'fixed_outs: {},', - str(any(isinstance(c, Register) - for c in r.outs)).lower()) + emit_operand_constraints( + r, r.ins, 'ins', tied_i2o, fixed_outs, fmt) + emit_operand_constraints( + r, r.outs, 'outs', tied_o2i, fixed_ins, fmt) + fmt.format('fixed_ins: {},', str(bool(fixed_ins)).lower()) + fmt.format('fixed_outs: {},', str(bool(fixed_outs)).lower()) fmt.format('tied_ops: {},', str(bool(tied_i2o)).lower()) fmt.format( 'clobbers_flags: {},', @@ -776,11 +773,16 @@ def emit_operand_constraints( seq, # type: Sequence[OperandConstraint] field, # type: str tied, # type: Dict[int, int] + fixops, # type: Set[Register] fmt # type: srcgen.Formatter ): # type: (...) -> None """ Emit a struct field initializer for an array of operand constraints. + + :param field: The name of the struct field to emit. + :param tied: Map of tied opnums to counterparts. + :param fix_ops: Set of fixed operands on the other side of the inst. """ if len(seq) == 0: fmt.line('{}: &[],'.format(field)) @@ -796,8 +798,9 @@ def emit_operand_constraints( fmt.format('regclass: &{}_DATA,', cons) elif isinstance(cons, Register): assert n not in tied, "Can't tie fixed register operand" - fmt.format( - 'kind: ConstraintKind::FixedReg({}),', cons.unit) + # See if this fixed register is also on the other side. + t = 'FixedTied' if cons in fixops else 'FixedReg' + fmt.format('kind: ConstraintKind::{}({}),', t, cons.unit) fmt.format('regclass: &{}_DATA,', cons.regclass) elif isinstance(cons, int): # This is a tied output constraint. It should never happen diff --git a/lib/cretonne/src/isa/constraints.rs b/lib/cretonne/src/isa/constraints.rs index aa3268187f..eb6b33c3cb 100644 --- a/lib/cretonne/src/isa/constraints.rs +++ b/lib/cretonne/src/isa/constraints.rs @@ -37,7 +37,8 @@ impl OperandConstraint { false } } - ConstraintKind::FixedReg(reg) => { + ConstraintKind::FixedReg(reg) | + ConstraintKind::FixedTied(reg) => { loc == ValueLoc::Reg(reg) && self.regclass.contains(reg) } ConstraintKind::Stack => { @@ -73,6 +74,13 @@ pub enum ConstraintKind { /// the out operand is `Tied(in)`. Tied(u8), + /// This operand must be a fixed register, and it has a tied counterpart. + /// + /// This works just like `FixedReg`, but additionally indicates that there are identical + /// input/output operands for this fixed register. For an input operand, this means that the + /// value will be clobbered by the instruction + FixedTied(RegUnit), + /// This operand must be a value in a stack slot. /// /// The constraint's `regclass` field is the register class that would normally be used to load diff --git a/lib/cretonne/src/regalloc/coloring.rs b/lib/cretonne/src/regalloc/coloring.rs index b6b403fb30..b44860c958 100644 --- a/lib/cretonne/src/regalloc/coloring.rs +++ b/lib/cretonne/src/regalloc/coloring.rs @@ -478,7 +478,8 @@ impl<'a> Context<'a> { // already in a register. let cur_reg = self.divert.reg(value, &self.cur.func.locations); match op.kind { - ConstraintKind::FixedReg(regunit) => { + ConstraintKind::FixedReg(regunit) | + ConstraintKind::FixedTied(regunit) => { if regunit != cur_reg { self.solver.reassign_in( value, @@ -532,7 +533,9 @@ impl<'a> Context<'a> { } } } - _ => {} + ConstraintKind::FixedReg(_) | + ConstraintKind::FixedTied(_) | + ConstraintKind::Stack => {} } } } @@ -674,8 +677,15 @@ impl<'a> Context<'a> { throughs: &[LiveValue], ) { for (op, lv) in constraints.iter().zip(defs) { - if let ConstraintKind::FixedReg(reg) = op.kind { - self.add_fixed_output(lv.value, op.regclass, reg, throughs); + match op.kind { + ConstraintKind::FixedReg(reg) | + ConstraintKind::FixedTied(reg) => { + self.add_fixed_output(lv.value, op.regclass, reg, throughs); + } + ConstraintKind::Reg | + ConstraintKind::Tied(_) | + ConstraintKind::Stack => {} + } } } @@ -741,6 +751,7 @@ impl<'a> Context<'a> { for (op, lv) in constraints.iter().zip(defs) { match op.kind { ConstraintKind::FixedReg(_) | + ConstraintKind::FixedTied(_) | ConstraintKind::Stack => continue, ConstraintKind::Reg => { self.solver.add_def(lv.value, op.regclass, !lv.is_local); diff --git a/lib/cretonne/src/regalloc/spilling.rs b/lib/cretonne/src/regalloc/spilling.rs index edda6f6f59..85d7b20b4a 100644 --- a/lib/cretonne/src/regalloc/spilling.rs +++ b/lib/cretonne/src/regalloc/spilling.rs @@ -301,6 +301,10 @@ impl<'a> Context<'a> { // A tied operand must kill the used value. reguse.tied = !lr.killed_at(inst, ebb, &self.cur.func.layout); } + ConstraintKind::FixedTied(_) => { + reguse.fixed = true; + reguse.tied = !lr.killed_at(inst, ebb, &self.cur.func.layout); + } ConstraintKind::Reg => {} } if lr.affinity.is_stack() {