From 3c99dc0eb48bee1bc2bbbec53632eb7ba46225c2 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Tue, 11 Apr 2017 16:53:54 -0700 Subject: [PATCH] Change dfg.inst_results to return a slice. Now we can access instruction results and arguments as well as EBB arguments as slices. Delete the Values iterator which was traversing the linked lists of values. It is no longer needed. --- lib/cretonne/meta/gen_instr.py | 13 ++-- lib/cretonne/src/ir/builder.rs | 2 +- lib/cretonne/src/ir/dfg.rs | 70 +++---------------- lib/cretonne/src/legalizer/boundary.rs | 12 ++-- .../src/regalloc/live_value_tracker.rs | 2 +- lib/cretonne/src/regalloc/liveness.rs | 2 +- lib/cretonne/src/verifier.rs | 6 +- lib/cretonne/src/write.rs | 2 +- lib/reader/src/parser.rs | 5 +- 9 files changed, 33 insertions(+), 81 deletions(-) diff --git a/lib/cretonne/meta/gen_instr.py b/lib/cretonne/meta/gen_instr.py index 9ce279dc52..40456339ae 100644 --- a/lib/cretonne/meta/gen_instr.py +++ b/lib/cretonne/meta/gen_instr.py @@ -693,14 +693,17 @@ def gen_inst_builder(inst, fmt): fmt.line(fcall + '.0') return + fmt.line('let (inst, dfg) = {};'.format(fcall)) + if len(inst.value_results) == 1: - fmt.line('Value::new_direct({}.0)'.format(fcall)) + fmt.line('dfg.first_result(inst)') return - fmt.line('let (inst, dfg) = {};'.format(fcall)) - fmt.line('let mut results = dfg.inst_results(inst);') - fmt.line('({})'.format(', '.join( - len(inst.value_results) * ['results.next().unwrap()']))) + fmt.format( + 'let results = &dfg.inst_results(inst)[0..{}];', + len(inst.value_results)) + fmt.format('({})', ', '.join( + 'results[{}]'.format(i) for i in range(len(inst.value_results)))) def gen_builder(insts, fmt): diff --git a/lib/cretonne/src/ir/builder.rs b/lib/cretonne/src/ir/builder.rs index dcec1bbb91..7ce771de58 100644 --- a/lib/cretonne/src/ir/builder.rs +++ b/lib/cretonne/src/ir/builder.rs @@ -143,7 +143,7 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> { assert_eq!(old_second_value, None, "Secondary result values {:?} would be left dangling by replacing {} with {}", - self.dfg.inst_results(self.inst).collect::>(), + self.dfg.inst_results(self.inst), self.dfg[self.inst].opcode(), data.opcode()); diff --git a/lib/cretonne/src/ir/dfg.rs b/lib/cretonne/src/ir/dfg.rs index 16de65fb3b..1778453ee2 100644 --- a/lib/cretonne/src/ir/dfg.rs +++ b/lib/cretonne/src/ir/dfg.rs @@ -314,43 +314,6 @@ enum ValueData { Alias { ty: Type, original: Value }, } -/// Iterate through a list of related value references, such as: -/// -/// - All results defined by an instruction. See `DataFlowGraph::inst_results`. -/// - All arguments to an EBB. See `DataFlowGraph::ebb_args`. -/// -/// A value iterator borrows a `DataFlowGraph` reference. -pub struct Values<'a> { - dfg: &'a DataFlowGraph, - cur: Option, -} - -impl<'a> Iterator for Values<'a> { - type Item = Value; - - fn next(&mut self) -> Option { - let rval = self.cur; - if let Some(prev) = rval { - // Advance self.cur to the next value, or `None`. - self.cur = match prev.expand() { - ExpandedValue::Direct(inst) => self.dfg.insts[inst].second_result(), - ExpandedValue::Table(index) => { - match self.dfg.extended_values[index] { - ValueData::Inst { next, .. } => next.into(), - ValueData::Arg { .. } => { - panic!("EBB argument {} appeared in value list", prev) - } - ValueData::Alias { .. } => { - panic!("Alias value {} appeared in value list", prev) - } - } - } - }; - } - rval - } -} - /// Instructions. /// impl DataFlowGraph { @@ -671,15 +634,8 @@ impl DataFlowGraph { } /// Iterate through all the results of an instruction. - pub fn inst_results(&self, inst: Inst) -> Values { - Values { - dfg: self, - cur: if self.insts[inst].first_type().is_void() { - None - } else { - Some(Value::new_direct(inst)) - }, - } + pub fn inst_results(&self, inst: Inst) -> &[Value] { + self.results[inst].as_slice(&self.value_lists) } /// Get the call signature of a direct or indirect call instruction. @@ -858,10 +814,9 @@ impl<'a> fmt::Display for DisplayInst<'a> { let dfg = self.0; let inst = &dfg[self.1]; - let mut results = dfg.inst_results(self.1); - if let Some(first) = results.next() { + if let Some((first, rest)) = dfg.inst_results(self.1).split_first() { write!(f, "{}", first)?; - for v in results { + for v in rest { write!(f, ", {}", v)?; } write!(f, " = ")?; @@ -904,11 +859,9 @@ mod tests { assert_eq!(ins.first_type(), types::I32); } - // Result iterator. - let mut res = dfg.inst_results(inst); - let val = res.next().unwrap(); - assert!(res.next().is_none()); - assert_eq!(val, dfg.first_result(inst)); + // Results. + let val = dfg.first_result(inst); + assert_eq!(dfg.inst_results(inst), &[val]); assert_eq!(dfg.value_def(val), ValueDef::Res(inst, 0)); assert_eq!(dfg.value_type(val), types::I32); @@ -925,9 +878,8 @@ mod tests { let inst = dfg.make_inst(idata); assert_eq!(dfg.display_inst(inst).to_string(), "trap"); - // Result iterator should be empty. - let mut res = dfg.inst_results(inst); - assert_eq!(res.next(), None); + // Result slice should be empty. + assert_eq!(dfg.inst_results(inst), &[]); } #[test] @@ -1028,8 +980,8 @@ mod tests { let new_iadd = dfg.redefine_first_value(pos); let new_s = dfg.first_result(new_iadd); assert_eq!(dfg[iadd].opcode(), Opcode::Copy); - assert_eq!(dfg.inst_results(iadd).collect::>(), [s]); - assert_eq!(dfg.inst_results(new_iadd).collect::>(), [new_s, c]); + assert_eq!(dfg.inst_results(iadd), &[s]); + assert_eq!(dfg.inst_results(new_iadd), &[new_s, c]); assert_eq!(dfg.resolve_copies(s), new_s); pos.next_inst(); diff --git a/lib/cretonne/src/legalizer/boundary.rs b/lib/cretonne/src/legalizer/boundary.rs index 42386cab83..f80b6574c4 100644 --- a/lib/cretonne/src/legalizer/boundary.rs +++ b/lib/cretonne/src/legalizer/boundary.rs @@ -302,11 +302,9 @@ fn convert_to_abi(dfg: &mut DataFlowGraph, } /// Check if a sequence of arguments match a desired sequence of argument types. -fn check_arg_types(dfg: &DataFlowGraph, args: Args, types: &[ArgumentType]) -> bool - where Args: IntoIterator -{ +fn check_arg_types(dfg: &DataFlowGraph, args: &[Value], types: &[ArgumentType]) -> bool { let mut n = 0; - for arg in args { + for &arg in args { match types.get(n) { Some(&ArgumentType { value_type, .. }) => { if dfg.value_type(arg) != value_type { @@ -335,7 +333,7 @@ fn check_call_signature(dfg: &DataFlowGraph, inst: Inst) -> Result<(), SigRef> { }; let sig = &dfg.signatures[sig_ref]; - if check_arg_types(dfg, args.iter().cloned(), &sig.argument_types[..]) && + if check_arg_types(dfg, args, &sig.argument_types[..]) && check_arg_types(dfg, dfg.inst_results(inst), &sig.return_types[..]) { // All types check out. Ok(()) @@ -347,9 +345,7 @@ fn check_call_signature(dfg: &DataFlowGraph, inst: Inst) -> Result<(), SigRef> { /// Check if the arguments of the return `inst` match the signature. fn check_return_signature(dfg: &DataFlowGraph, inst: Inst, sig: &Signature) -> bool { - check_arg_types(dfg, - dfg.inst_variable_args(inst).iter().cloned(), - &sig.return_types) + check_arg_types(dfg, dfg.inst_variable_args(inst), &sig.return_types) } /// Insert ABI conversion code for the arguments to the call or return instruction at `pos`. diff --git a/lib/cretonne/src/regalloc/live_value_tracker.rs b/lib/cretonne/src/regalloc/live_value_tracker.rs index 458fe14e26..c9f5146245 100644 --- a/lib/cretonne/src/regalloc/live_value_tracker.rs +++ b/lib/cretonne/src/regalloc/live_value_tracker.rs @@ -233,7 +233,7 @@ impl LiveValueTracker { // Add the values defined by `inst`. let first_def = self.live.values.len(); - for value in dfg.inst_results(inst) { + for &value in dfg.inst_results(inst) { let lr = match liveness.get(value) { Some(lr) => lr, None => panic!("{} result {} has no live range", dfg[inst].opcode(), value), diff --git a/lib/cretonne/src/regalloc/liveness.rs b/lib/cretonne/src/regalloc/liveness.rs index 90b817a825..cb937d67a2 100644 --- a/lib/cretonne/src/regalloc/liveness.rs +++ b/lib/cretonne/src/regalloc/liveness.rs @@ -308,7 +308,7 @@ impl Liveness { // Make sure we have created live ranges for dead defs. // TODO: When we implement DCE, we can use the absence of a live range to indicate // an unused value. - for def in func.dfg.inst_results(inst) { + for &def in func.dfg.inst_results(inst) { get_or_create(&mut self.ranges, def, func, &enc_info); } diff --git a/lib/cretonne/src/verifier.rs b/lib/cretonne/src/verifier.rs index ace83235df..d6018a8f5c 100644 --- a/lib/cretonne/src/verifier.rs +++ b/lib/cretonne/src/verifier.rs @@ -193,7 +193,7 @@ impl<'a> Verifier<'a> { } } else { // All result values for multi-valued instructions are created - let got_results = dfg.inst_results(inst).count(); + let got_results = dfg.inst_results(inst).len(); if got_results != total_results { return err!(inst, "expected {} result values, found {}", @@ -212,7 +212,7 @@ impl<'a> Verifier<'a> { self.verify_value(inst, arg)?; } - for res in self.func.dfg.inst_results(inst) { + for &res in self.func.dfg.inst_results(inst) { self.verify_value(inst, res)?; } @@ -448,7 +448,7 @@ impl<'a> Verifier<'a> { fn typecheck_results(&self, inst: Inst, ctrl_type: Type) -> Result<()> { let mut i = 0; - for result in self.func.dfg.inst_results(inst) { + for &result in self.func.dfg.inst_results(inst) { let result_type = self.func.dfg.value_type(result); let expected_type = self.func.dfg.compute_result_type(inst, i, ctrl_type); if let Some(expected_type) = expected_type { diff --git a/lib/cretonne/src/write.rs b/lib/cretonne/src/write.rs index 09a4410f43..ae4a38284f 100644 --- a/lib/cretonne/src/write.rs +++ b/lib/cretonne/src/write.rs @@ -188,7 +188,7 @@ fn write_instruction(w: &mut Write, // Write value locations, if we have them. if !func.locations.is_empty() { let regs = isa.register_info(); - for r in func.dfg.inst_results(inst) { + for &r in func.dfg.inst_results(inst) { write!(s, ",{}", func.locations diff --git a/lib/reader/src/parser.rs b/lib/reader/src/parser.rs index a5c3b4244c..5f51b4c342 100644 --- a/lib/reader/src/parser.rs +++ b/lib/reader/src/parser.rs @@ -1257,12 +1257,13 @@ impl<'a> Parser<'a> { // holds a reference to `ctx.function`. self.add_values(&mut ctx.map, results.into_iter(), - ctx.function.dfg.inst_results(inst))?; + ctx.function.dfg.inst_results(inst).iter().cloned())?; if let Some(result_locations) = result_locations { - for (value, loc) in ctx.function + for (&value, loc) in ctx.function .dfg .inst_results(inst) + .iter() .zip(result_locations) { *ctx.function.locations.ensure(value) = loc; }