From 6587784d7de1db5373f6f194bdab024e9fdc8616 Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Fri, 8 Jul 2016 15:15:53 -0700 Subject: [PATCH] Rewrite EBB and value references after parsing. We llow forward references to values and EBBs, so it is not possible to rewrite these from the source domain to the in-memory domain during parsing. Instead go through all the instructions after parsing everything and rewrite the value and EBB references when everything has been created and mapped. --- cranelift/src/libcretonne/instructions.rs | 18 +++- cranelift/src/libcretonne/repr.rs | 15 ++-- cranelift/src/libreader/parser.rs | 105 ++++++++++++++++++++++ cranelift/tests/parser/rewrite.cton | 24 +++++ cranelift/tests/parser/rewrite.cton.ref | 13 +++ 5 files changed, 168 insertions(+), 7 deletions(-) create mode 100644 cranelift/tests/parser/rewrite.cton create mode 100644 cranelift/tests/parser/rewrite.cton.ref diff --git a/cranelift/src/libcretonne/instructions.rs b/cranelift/src/libcretonne/instructions.rs index f6ca187511..9a87e85d71 100644 --- a/cranelift/src/libcretonne/instructions.rs +++ b/cranelift/src/libcretonne/instructions.rs @@ -8,6 +8,7 @@ use std::fmt::{self, Display, Formatter}; use std::str::FromStr; +use std::ops::{Deref, DerefMut}; use entities::*; use immediates::*; @@ -230,6 +231,21 @@ impl VariableArgs { } } +// Coerce VariableArgs into a &[Value] slice. +impl Deref for VariableArgs { + type Target = [Value]; + + fn deref<'a>(&'a self) -> &'a [Value] { + &self.0 + } +} + +impl DerefMut for VariableArgs { + fn deref_mut<'a>(&'a mut self) -> &'a mut [Value] { + &mut self.0 + } +} + impl Display for VariableArgs { fn fmt(&self, fmt: &mut Formatter) -> fmt::Result { try!(write!(fmt, "(")); @@ -294,7 +310,7 @@ pub struct CallData { second_result: Value, // Dynamically sized array containing call argument values. - arguments: VariableArgs, + pub arguments: VariableArgs, } impl Display for CallData { diff --git a/cranelift/src/libcretonne/repr.rs b/cranelift/src/libcretonne/repr.rs index ff8d5d4dee..5a36115fbd 100644 --- a/cranelift/src/libcretonne/repr.rs +++ b/cranelift/src/libcretonne/repr.rs @@ -101,10 +101,6 @@ impl Function { inst } - fn inst_mut(&mut self, inst: Inst) -> &mut InstructionData { - &mut self.instructions[inst.index()] - } - /// Create result values for an instruction that produces multiple results. /// /// Instructions that produce 0 or 1 result values only need to be created with `make_inst`. If @@ -147,11 +143,11 @@ impl Function { // Update the second_result pointer in `inst`. if head != NO_VALUE { - *self.inst_mut(inst) + *self[inst] .second_result_mut() .expect("instruction format doesn't allow multiple results") = head; } - *self.inst_mut(inst).first_type_mut() = first_type; + *self[inst].first_type_mut() = first_type; fixed_results } @@ -456,6 +452,13 @@ impl Index for Function { } } +/// Allow mutable access to instructions via function indexing. +impl IndexMut for Function { + fn index_mut<'a>(&'a mut self, inst: Inst) -> &'a mut InstructionData { + &mut self.instructions[inst.index()] + } +} + /// A node in a double linked list of instructions is a basic block. #[derive(Debug)] struct InstNode { diff --git a/cranelift/src/libreader/parser.rs b/cranelift/src/libreader/parser.rs index 24d46b884d..e55a277d18 100644 --- a/cranelift/src/libreader/parser.rs +++ b/cranelift/src/libreader/parser.rs @@ -73,6 +73,9 @@ struct Context { stack_slots: HashMap, // ssNN ebbs: HashMap, // ebbNN values: HashMap, // vNN, vxNN + + // Remember the location of every instruction. + inst_locs: Vec<(Inst, Location)>, } impl Context { @@ -82,6 +85,7 @@ impl Context { stack_slots: HashMap::new(), ebbs: HashMap::new(), values: HashMap::new(), + inst_locs: Vec::new(), } } @@ -112,6 +116,101 @@ impl Context { Ok(()) } } + + // Record the location of an instuction. + fn add_inst_loc(&mut self, inst: Inst, loc: &Location) { + self.inst_locs.push((inst, *loc)); + } + + // The parser creates all instructions with Ebb and Value references using the source file + // numbering. These references need to be rewritten after parsing is complete since forward + // references are allowed. + + // Rewrite an Ebb reference. + fn rewrite_ebb(map: &HashMap, ebb: &mut Ebb, loc: &Location) -> Result<()> { + match map.get(ebb) { + Some(&new) => { + *ebb = new; + Ok(()) + } + None => err!(loc, "undefined reference: {}", ebb), + } + } + + // Rewrite a value reference. + fn rewrite_value(map: &HashMap, val: &mut Value, loc: &Location) -> Result<()> { + match map.get(val) { + Some(&new) => { + *val = new; + Ok(()) + } + None => err!(loc, "undefined reference: {}", val), + } + } + + // Rewrite a slice of value references. + fn rewrite_values(map: &HashMap, + vals: &mut [Value], + loc: &Location) + -> Result<()> { + for val in vals { + try!(Self::rewrite_value(map, val, loc)); + } + Ok(()) + } + + // Rewrite all EBB and value references in the function. + fn rewrite_references(&mut self) -> Result<()> { + for &(inst, loc) in &self.inst_locs { + match self.function[inst] { + InstructionData::Nullary { .. } | + InstructionData::UnaryImm { .. } | + InstructionData::UnaryIeee32 { .. } | + InstructionData::UnaryIeee64 { .. } | + InstructionData::UnaryImmVector { .. } => {} + + InstructionData::Unary { ref mut arg, .. } | + InstructionData::BinaryImm { ref mut arg, .. } | + InstructionData::BinaryImmRev { ref mut arg, .. } | + InstructionData::ExtractLane { ref mut arg, .. } | + InstructionData::BranchTable { ref mut arg, .. } => { + try!(Self::rewrite_value(&self.values, arg, &loc)); + } + + InstructionData::Binary { ref mut args, .. } | + InstructionData::BinaryOverflow { ref mut args, .. } | + InstructionData::InsertLane { ref mut args, .. } | + InstructionData::IntCompare { ref mut args, .. } | + InstructionData::FloatCompare { ref mut args, .. } => { + try!(Self::rewrite_values(&self.values, args, &loc)); + } + + InstructionData::Ternary { ref mut args, .. } => { + try!(Self::rewrite_values(&self.values, args, &loc)); + } + + InstructionData::Jump { ref mut data, .. } => { + try!(Self::rewrite_ebb(&self.ebbs, &mut data.destination, &loc)); + try!(Self::rewrite_values(&self.values, &mut data.arguments, &loc)); + } + + InstructionData::Branch { ref mut data, .. } => { + try!(Self::rewrite_value(&self.values, &mut data.arg, &loc)); + try!(Self::rewrite_ebb(&self.ebbs, &mut data.destination, &loc)); + try!(Self::rewrite_values(&self.values, &mut data.arguments, &loc)); + } + + InstructionData::Call { ref mut data, .. } => { + try!(Self::rewrite_values(&self.values, &mut data.arguments, &loc)); + } + } + } + + // TODO: Rewrite EBB references in jump tables. (Once jump table data structures are + // defined). + + Ok(()) + } } impl<'a> Parser<'a> { @@ -323,6 +422,10 @@ impl<'a> Parser<'a> { // function ::= function-spec "{" preamble function-body * "}" try!(self.match_token(Token::RBrace, "expected '}' after function body")); + // Rewrite references to values and EBBs after parsing everuthing to allow forward + // references. + try!(ctx.rewrite_references()); + Ok(ctx.function) } @@ -575,6 +678,7 @@ impl<'a> Parser<'a> { } else { return err!(self.loc, "expected instruction opcode"); }; + let opcode_loc = self.loc; self.consume(); // Look for a controlling type variable annotation. @@ -597,6 +701,7 @@ impl<'a> Parser<'a> { let inst = ctx.function.make_inst(inst_data); let num_results = ctx.function.make_inst_results(inst, ctrl_typevar); ctx.function.append_inst(ebb, inst); + ctx.add_inst_loc(inst, &opcode_loc); if results.len() != num_results { return err!(self.loc, diff --git a/cranelift/tests/parser/rewrite.cton b/cranelift/tests/parser/rewrite.cton new file mode 100644 index 0000000000..33e5277db6 --- /dev/null +++ b/cranelift/tests/parser/rewrite.cton @@ -0,0 +1,24 @@ +; The .cton parser can't preserve the actual entity numbers in the input file +; since entities are numbered as they are created. For entities declared in the +; preamble, this is no problem, but for EBB and value references, mapping +; source numbers to real numbers can be a problem. +; +; It is possible to refer to instructions and EBBs that have not yet been +; defined in the lexical order, so the parser needs to rewrite these references +; after the fact. + +; Check that defining numbers are rewritten. +function defs() { +ebb100(v20: i32): + v1000 = iconst.i32x8 5 + vx200 = f64const 0x4.0p0 + trap +} + +; Using values. +function use_value() { +ebb100(v20: i32): + v1000 = iadd_imm v20, 5 + vx200 = iadd v20, v1000 + jump ebb100(v1000) +} diff --git a/cranelift/tests/parser/rewrite.cton.ref b/cranelift/tests/parser/rewrite.cton.ref new file mode 100644 index 0000000000..8f379abd64 --- /dev/null +++ b/cranelift/tests/parser/rewrite.cton.ref @@ -0,0 +1,13 @@ +function defs() { +ebb0(vx0: i32): + v0 = iconst.i32x8 5 + v1 = f64const 0x1.0000000000000p2 + trap +} + +function "use_value"() { +ebb0(vx0: i32): + v0 = iadd_imm vx0, 5 + v1 = iadd vx0, v0 + jump ebb0(v0) +}