From 71338bb31f9c04024111b957c4940ada3bb04564 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Wed, 12 Apr 2017 08:34:59 -0700 Subject: [PATCH] Simplify the back-end of InstBuilder. We don't want to distinguish between single-result and multiple-result instructions any longer. - Merge the simple_instruction() and complex_instruction() builder methods into a single build() that can handle all cases. - All format constructors now take a ctrl_type argument. Previously, some would take a result_type argument. - Instruction constructors no longer attempt to compute a single result type. Just pass a ctrl_type and let the backend decide. Fix one format constructor call in legalizer/split.rs which now takes a ctrl_type instead of a result type. --- lib/cretonne/meta/gen_instr.py | 56 +++++-------------- lib/cretonne/src/ir/builder.rs | 83 ++++++----------------------- lib/cretonne/src/ir/dfg.rs | 7 ++- lib/cretonne/src/legalizer/split.rs | 2 +- 4 files changed, 36 insertions(+), 112 deletions(-) diff --git a/lib/cretonne/meta/gen_instr.py b/lib/cretonne/meta/gen_instr.py index 40456339ae..ece67dfb2f 100644 --- a/lib/cretonne/meta/gen_instr.py +++ b/lib/cretonne/meta/gen_instr.py @@ -498,21 +498,12 @@ def gen_format_constructor(iform, fmt): Emit a method for creating and inserting inserting an `iform` instruction, where `iform` is an instruction format. - Instruction formats that can produce multiple results take a `ctrl_typevar` - argument for deducing the result types. Others take a `result_type` - argument. + All instruction formats take an `opcode` argument and a `ctrl_typevar` + argument for deducing the result types. """ # Construct method arguments. - args = ['self', 'opcode: Opcode'] - - if iform.multiple_results: - args.append('ctrl_typevar: Type') - # `dfg::make_inst_results` will compute the result type. - result_type = 'types::VOID' - else: - args.append('result_type: Type') - result_type = 'result_type' + args = ['self', 'opcode: Opcode', 'ctrl_typevar: Type'] # Normal operand arguments. Start with the immediate operands. for f in iform.imm_fields: @@ -537,16 +528,12 @@ def gen_format_constructor(iform, fmt): with fmt.indented( 'let data = InstructionData::{} {{'.format(iform.name), '};'): fmt.line('opcode: opcode,') - fmt.line('ty: {},'.format(result_type)) + fmt.line('ty: types::VOID,') if iform.multiple_results: fmt.line('second_result: None.into(),') gen_member_inits(iform, fmt) - # Create result values if necessary. - if iform.multiple_results: - fmt.line('self.complex_instruction(data, ctrl_typevar)') - else: - fmt.line('self.simple_instruction(data)') + fmt.line('self.build(data, ctrl_typevar)') def gen_member_inits(iform, fmt): @@ -631,34 +618,19 @@ def gen_inst_builder(inst, fmt): if inst.is_polymorphic and not inst.use_typevar_operand: # This was an explicit method argument. args.append(inst.ctrl_typevar.name) - elif len(inst.value_results) == 0: + elif len(inst.value_results) == 0 or not inst.is_polymorphic: + # No controlling type variable needed. args.append('types::VOID') - elif inst.is_polymorphic: + else: + assert inst.is_polymorphic and inst.use_typevar_operand # Infer the controlling type variable from the input operands. opnum = inst.value_opnums[inst.format.typevar_operand] fmt.line( 'let ctrl_typevar = self.data_flow_graph().value_type({});' .format(inst.ins[opnum].name)) - if inst.format.multiple_results: - # The format constructor will resolve the result types from the - # type var. - args.append('ctrl_typevar') - elif inst.outs[inst.value_results[0]].typevar == inst.ctrl_typevar: - # The format constructor expects a simple result type. - # No type transformation needed from the controlling type - # variable. - args.append('ctrl_typevar') - else: - # The format constructor expects a simple result type. - # TODO: This formula could be resolved ahead of time. - args.append( - 'Opcode::{}.constraints().result_type(0, ctrl_typevar)' - .format(inst.camel_name)) - else: - # This non-polymorphic instruction has a fixed result type. - args.append( - inst.outs[inst.value_results[0]] - .typevar.singleton_type.rust_name()) + # The format constructor will resolve the result types from the + # type var. + args.append('ctrl_typevar') # Now add all of the immediate operands to the constructor arguments. for opnum in inst.imm_opnums: @@ -717,8 +689,8 @@ def gen_builder(insts, fmt): The `InstrBuilder` trait has one method per instruction opcode for conveniently constructing the instruction with minimum arguments. Polymorphic instructions infer their result types from the input - arguments when possible. In some cases, an explicit `result_type` - or `ctrl_typevar` argument is required. + arguments when possible. In some cases, an explicit `ctrl_typevar` + argument is required. The opcode methods return the new instruction's result values, or the `Inst` itself for instructions that don't have any results. diff --git a/lib/cretonne/src/ir/builder.rs b/lib/cretonne/src/ir/builder.rs index 7ce771de58..c64ce79dc5 100644 --- a/lib/cretonne/src/ir/builder.rs +++ b/lib/cretonne/src/ir/builder.rs @@ -24,21 +24,11 @@ pub trait InstBuilderBase<'f>: Sized { fn data_flow_graph(&self) -> &DataFlowGraph; fn data_flow_graph_mut(&mut self) -> &mut DataFlowGraph; - /// Insert a simple instruction and return a reference to it. + /// Insert an instruction and return a reference to it, consuming the builder. /// - /// A 'simple' instruction has at most one result, and the `data.ty` field must contain the - /// result type or `VOID` for an instruction with no result values. - fn simple_instruction(self, data: InstructionData) -> (Inst, &'f mut DataFlowGraph); - - /// Insert a simple instruction and return a reference to it. - /// - /// A 'complex' instruction may produce multiple results, and the result types may depend on a - /// controlling type variable. For non-polymorphic instructions with multiple results, pass - /// `VOID` for the `ctrl_typevar` argument. - fn complex_instruction(self, - data: InstructionData, - ctrl_typevar: Type) - -> (Inst, &'f mut DataFlowGraph); + /// The result types may depend on a controlling type variable. For non-polymorphic + /// instructions with multiple results, pass `VOID` for the `ctrl_typevar` argument. + fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph); } // Include trait code generated by `lib/cretonne/meta/gen_instr.py`. @@ -79,16 +69,7 @@ impl<'c, 'fc, 'fd> InstBuilderBase<'fd> for InsertBuilder<'c, 'fc, 'fd> { self.dfg } - fn simple_instruction(self, data: InstructionData) -> (Inst, &'fd mut DataFlowGraph) { - let inst = self.dfg.make_inst(data); - self.pos.insert_inst(inst); - (inst, self.dfg) - } - - fn complex_instruction(self, - data: InstructionData, - ctrl_typevar: Type) - -> (Inst, &'fd mut DataFlowGraph) { + fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'fd mut DataFlowGraph) { let inst = self.dfg.make_inst(data); self.dfg.make_inst_results(inst, ctrl_typevar); self.pos.insert_inst(inst); @@ -98,20 +79,12 @@ impl<'c, 'fc, 'fd> InstBuilderBase<'fd> for InsertBuilder<'c, 'fc, 'fd> { /// Instruction builder that replaces an existing instruction. /// -/// The inserted instruction will have the same `Inst` number as the old one. This is the only way -/// of rewriting the first result value of an instruction since this is a `ExpandedValue::Direct` -/// variant which encodes the instruction number directly. +/// The inserted instruction will have the same `Inst` number as the old one. /// -/// If the old instruction produced a value, the same value number will refer to the new -/// instruction's first result, so if that value has any uses the type should stay the same. -/// -/// If the old instruction still has secondary result values attached, it is assumed that the new -/// instruction produces the same number and types of results. The old secondary values are -/// preserved. If the replacement instruction format does not support multiple results, the builder -/// panics. It is a bug to leave result values dangling. -/// -/// If the old instruction was capable of producing secondary results, but the values have been -/// detached, new result values are generated by calling `DataFlowGraph::make_inst_results()`. +/// If the old instruction still has result values attached, it is assumed that the new instruction +/// produces the same number and types of results. The old result values are preserved. If the +/// replacement instruction format does not support multiple results, the builder panics. It is a +/// bug to leave result values dangling. pub struct ReplaceBuilder<'f> { dfg: &'f mut DataFlowGraph, inst: Inst, @@ -136,46 +109,20 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> { self.dfg } - fn simple_instruction(self, data: InstructionData) -> (Inst, &'f mut DataFlowGraph) { - // The replacement instruction cannot generate multiple results, so verify that the old - // instruction's secondary results have been detached. - let old_second_value = self.dfg[self.inst].second_result(); - assert_eq!(old_second_value, - None, - "Secondary result values {:?} would be left dangling by replacing {} with {}", - self.dfg.inst_results(self.inst), - self.dfg[self.inst].opcode(), - data.opcode()); - - // Splat the new instruction on top of the old one. - self.dfg[self.inst] = data; - (self.inst, self.dfg) - } - - fn complex_instruction(self, - data: InstructionData, - ctrl_typevar: Type) - -> (Inst, &'f mut DataFlowGraph) { - // If the old instruction still has secondary results attached, we'll keep them. - let old_second_value = self.dfg[self.inst].second_result(); - + fn build(self, data: InstructionData, ctrl_typevar: Type) -> (Inst, &'f mut DataFlowGraph) { // Splat the new instruction on top of the old one. self.dfg[self.inst] = data; - if old_second_value.is_none() { - // The old secondary values were either detached or non-existent. - // Construct new ones and set the first result type too. + if !self.dfg.has_results(self.inst) { + // The old result values were either detached or non-existent. + // Construct new ones. self.dfg.make_inst_results(self.inst, ctrl_typevar); } else { // Reattach the old secondary values. + let old_second_value = self.dfg.inst_results(self.inst).get(1).cloned(); if let Some(val_ref) = self.dfg[self.inst].second_result_mut() { // Don't check types here. Leave that to the verifier. *val_ref = old_second_value.into(); - } else { - // Actually, this instruction format should have called `simple_instruction()`, but - // we don't have a rule against calling `complex_instruction()` even when it is - // overkill. - panic!("Secondary result values left dangling"); } // Normally, make_inst_results() would also set the first result type, but we're not diff --git a/lib/cretonne/src/ir/dfg.rs b/lib/cretonne/src/ir/dfg.rs index 1778453ee2..6fe3f91d57 100644 --- a/lib/cretonne/src/ir/dfg.rs +++ b/lib/cretonne/src/ir/dfg.rs @@ -633,7 +633,12 @@ impl DataFlowGraph { .expect("Instruction has no results") } - /// Iterate through all the results of an instruction. + /// Test if `inst` has any result values currently. + pub fn has_results(&self, inst: Inst) -> bool { + !self.results[inst].is_empty() + } + + /// Return all the results of an instruction. pub fn inst_results(&self, inst: Inst) -> &[Value] { self.results[inst].as_slice(&self.value_lists) } diff --git a/lib/cretonne/src/legalizer/split.rs b/lib/cretonne/src/legalizer/split.rs index d1f55a6266..3c03d1d77b 100644 --- a/lib/cretonne/src/legalizer/split.rs +++ b/lib/cretonne/src/legalizer/split.rs @@ -228,7 +228,7 @@ fn split_value(dfg: &mut DataFlowGraph, // need to insert a split instruction before returning. pos.goto_top(ebb); pos.next_inst(); - let concat_inst = dfg.ins(pos).Binary(concat, ty, lo, hi).0; + let concat_inst = dfg.ins(pos).Binary(concat, split_type, lo, hi).0; let concat_val = dfg.first_result(concat_inst); dfg.change_to_alias(value, concat_val);