From 1098eafb45427f247e4d652cd02196b7b71e9a40 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Thu, 4 Oct 2018 10:35:31 -0700 Subject: [PATCH] Remove the concept of non-dense jump tables. WebAssembly doesn't have non-dense jump tables, and higher-level users are better served by the facilities in lib/frontend/src/switch.rs for working with non-dense switches. This eliminates the concept of "absent" jump table entries, which were represented as "0" in the text format. Also, jump table contents are now enclosed in `[` and `]`, so that we can unambiguously display empty jump tables. Previously, empty jump tables were displayed as if they had a single absent entry. --- cranelift/docs/ir.rst | 5 +- cranelift/filetests/isa/x86/binary64.clif | 2 +- .../filetests/isa/x86/legalize-br-table.clif | 2 +- cranelift/filetests/parser/branch.clif | 8 +- cranelift/filetests/verifier/type_check.clif | 2 +- cranelift/filetests/wasm/control.clif | 2 +- lib/codegen/src/binemit/mod.rs | 11 +- lib/codegen/src/dominator_tree.rs | 4 +- lib/codegen/src/flowgraph.rs | 4 +- lib/codegen/src/ir/function.rs | 5 - lib/codegen/src/ir/jumptable.rs | 140 +++++------------- lib/codegen/src/legalizer/mod.rs | 13 +- lib/codegen/src/regalloc/coloring.rs | 4 +- lib/codegen/src/verifier/flags.rs | 4 +- lib/codegen/src/verifier/locations.rs | 4 +- lib/codegen/src/verifier/mod.rs | 4 +- lib/filetests/src/test_binemit.rs | 11 +- lib/frontend/src/frontend.rs | 12 +- lib/frontend/src/ssa.rs | 10 +- lib/frontend/src/switch.rs | 6 +- lib/reader/src/parser.rs | 66 ++++----- lib/reader/src/sourcemap.rs | 2 +- 22 files changed, 113 insertions(+), 208 deletions(-) diff --git a/cranelift/docs/ir.rst b/cranelift/docs/ir.rst index 8bd17cacbb..c452dfb6bc 100644 --- a/cranelift/docs/ir.rst +++ b/cranelift/docs/ir.rst @@ -360,13 +360,12 @@ instruction in the EBB. .. autoinst:: brff .. autoinst:: br_table -.. inst:: JT = jump_table EBB0, EBB1, ..., EBBn +.. inst:: JT = jump_table [EBB0, EBB1, ..., EBBn] Declare a jump table in the :term:`function preamble`. This declares a jump table for use by the :inst:`br_table` indirect branch - instruction. Entries in the table are either EBB names, or ``0`` which - indicates an absent entry. + instruction. Entries in the table are EBB names. The EBBs listed must belong to the current function, and they can't have any arguments. diff --git a/cranelift/filetests/isa/x86/binary64.clif b/cranelift/filetests/isa/x86/binary64.clif index 9ea203b799..dca189d406 100644 --- a/cranelift/filetests/isa/x86/binary64.clif +++ b/cranelift/filetests/isa/x86/binary64.clif @@ -1405,7 +1405,7 @@ ebb0: ; Tests for i64 jump table instructions. function %I64_JT(i64 [%rdi]) { - jt0 = jump_table ebb1, ebb2, ebb3 + jt0 = jump_table [ebb1, ebb2, ebb3] ebb0(v0: i64 [%rdi]): ; Note: The next two lines will need to change whenever instructions are diff --git a/cranelift/filetests/isa/x86/legalize-br-table.clif b/cranelift/filetests/isa/x86/legalize-br-table.clif index b600de147c..959d3028b2 100644 --- a/cranelift/filetests/isa/x86/legalize-br-table.clif +++ b/cranelift/filetests/isa/x86/legalize-br-table.clif @@ -4,7 +4,7 @@ target x86_64 function u0:0(i64) system_v { ss0 = explicit_slot 1 - jt0 = jump_table ebb1 + jt0 = jump_table [ebb1] ebb0(v0: i64): v1 = stack_addr.i64 ss0 diff --git a/cranelift/filetests/parser/branch.clif b/cranelift/filetests/parser/branch.clif index ba68fc590d..4404feba14 100644 --- a/cranelift/filetests/parser/branch.clif +++ b/cranelift/filetests/parser/branch.clif @@ -81,8 +81,8 @@ ebb1(v92: i32, v93: f32): ; nextln: } function %jumptable(i32) { - jt200 = jump_table 0, 0 - jt2 = jump_table 0, 0, ebb10, ebb40, ebb20, ebb30 + jt200 = jump_table [] + jt2 = jump_table [ebb10, ebb40, ebb20, ebb30] ebb10(v3: i32): br_table v3, ebb50, jt2 @@ -97,8 +97,8 @@ ebb50: trap user1 } ; sameln: function %jumptable(i32) fast { -; check: jt2 = jump_table 0, 0, ebb10, ebb40, ebb20, ebb30 -; check: jt200 = jump_table 0 +; check: jt2 = jump_table [ebb10, ebb40, ebb20, ebb30] +; check: jt200 = jump_table [] ; check: ebb10(v3: i32): ; nextln: br_table v3, ebb50, jt2 ; nextln: diff --git a/cranelift/filetests/verifier/type_check.clif b/cranelift/filetests/verifier/type_check.clif index 9fa8fa9cca..ce61b81a10 100644 --- a/cranelift/filetests/verifier/type_check.clif +++ b/cranelift/filetests/verifier/type_check.clif @@ -68,7 +68,7 @@ function %fn_call_incorrect_arg_type(i64) { ; TODO: Should we instead just verify that jump tables contain no EBBs that take arguments? This ; error doesn't occur if no instruction uses the jump table. function %jump_table_args() { - jt1 = jump_table ebb1 + jt1 = jump_table [ebb1] ebb0: v0 = iconst.i32 0 br_table v0, ebb2, jt1 ; error: takes no arguments, but had target ebb1 with 1 arguments diff --git a/cranelift/filetests/wasm/control.clif b/cranelift/filetests/wasm/control.clif index 2e9f9a17ad..66c82df937 100644 --- a/cranelift/filetests/wasm/control.clif +++ b/cranelift/filetests/wasm/control.clif @@ -48,7 +48,7 @@ ebb0: } function %br_table(i32) { -jt0 = jump_table ebb3, ebb1, 0, ebb2 +jt0 = jump_table [ebb3, ebb1, ebb2] ebb0(v0: i32): br_table v0, ebb4, jt0 diff --git a/lib/codegen/src/binemit/mod.rs b/lib/codegen/src/binemit/mod.rs index 42b16d442b..661297a9ee 100644 --- a/lib/codegen/src/binemit/mod.rs +++ b/lib/codegen/src/binemit/mod.rs @@ -132,14 +132,9 @@ where // output jump tables for (jt, jt_data) in func.jump_tables.iter() { let jt_offset = func.jt_offsets[jt]; - for idx in 0..jt_data.len() { - match jt_data.get_entry(idx) { - Some(ebb) => { - let rel_offset: i32 = func.offsets[ebb] as i32 - jt_offset as i32; - sink.put4(rel_offset as u32) - } - None => sink.put4(0), - } + for ebb in jt_data.iter() { + let rel_offset: i32 = func.offsets[*ebb] as i32 - jt_offset as i32; + sink.put4(rel_offset as u32) } } } diff --git a/lib/codegen/src/dominator_tree.rs b/lib/codegen/src/dominator_tree.rs index 9883e34418..e25e11990c 100644 --- a/lib/codegen/src/dominator_tree.rs +++ b/lib/codegen/src/dominator_tree.rs @@ -347,8 +347,8 @@ impl DominatorTree { match func.dfg.analyze_branch(inst) { BranchInfo::SingleDest(succ, _) => self.push_if_unseen(succ), BranchInfo::Table(jt, dest) => { - for (_, succ) in func.jump_tables[jt].entries() { - self.push_if_unseen(succ); + for succ in func.jump_tables[jt].iter() { + self.push_if_unseen(*succ); } if let Some(dest) = dest { self.push_if_unseen(dest); diff --git a/lib/codegen/src/flowgraph.rs b/lib/codegen/src/flowgraph.rs index 48b328cb51..3f61f6a852 100644 --- a/lib/codegen/src/flowgraph.rs +++ b/lib/codegen/src/flowgraph.rs @@ -129,8 +129,8 @@ impl ControlFlowGraph { if let Some(dest) = dest { self.add_edge(ebb, inst, dest); } - for (_, dest) in func.jump_tables[jt].entries() { - self.add_edge(ebb, inst, dest); + for dest in func.jump_tables[jt].iter() { + self.add_edge(ebb, inst, *dest); } } BranchInfo::NotABranch => {} diff --git a/lib/codegen/src/ir/function.rs b/lib/codegen/src/ir/function.rs index 26e860dbe4..99516d9711 100644 --- a/lib/codegen/src/ir/function.rs +++ b/lib/codegen/src/ir/function.rs @@ -122,11 +122,6 @@ impl Function { self.jump_tables.push(data) } - /// Inserts an entry in a previously declared jump table. - pub fn insert_jump_table_entry(&mut self, jt: JumpTable, index: usize, ebb: Ebb) { - self.jump_tables[jt].set_entry(index, ebb); - } - /// Creates a stack slot in the function, to be used by `stack_load`, `stack_store` and /// `stack_addr` instructions. pub fn create_stack_slot(&mut self, data: StackSlotData) -> StackSlot { diff --git a/lib/codegen/src/ir/jumptable.rs b/lib/codegen/src/ir/jumptable.rs index aff6503a57..7ba0a7d13c 100644 --- a/lib/codegen/src/ir/jumptable.rs +++ b/lib/codegen/src/ir/jumptable.rs @@ -4,39 +4,29 @@ //! The actual table of destinations is stored in a `JumpTableData` struct defined in this module. use ir::entities::Ebb; -use packed_option::PackedOption; use std::fmt::{self, Display, Formatter}; -use std::iter; -use std::slice; +use std::slice::{Iter, IterMut}; use std::vec::Vec; /// Contents of a jump table. /// -/// All jump tables use 0-based indexing and are expected to be densely populated. They don't need -/// to be completely populated, though. Individual entries can be missing. +/// All jump tables use 0-based indexing and densely populated. #[derive(Clone)] pub struct JumpTableData { - // Table entries, using `None` as a placeholder for missing entries. - table: Vec>, - - // How many `None` holes in table? - holes: usize, + // Table entries. + table: Vec, } impl JumpTableData { /// Create a new empty jump table. pub fn new() -> Self { - Self { - table: Vec::new(), - holes: 0, - } + Self { table: Vec::new() } } /// Create a new empty jump table with the specified capacity. pub fn with_capacity(capacity: usize) -> Self { Self { table: Vec::with_capacity(capacity), - holes: 0, } } @@ -45,100 +35,48 @@ impl JumpTableData { self.table.len() } - /// Boolean that is false if the table has missing entries. - pub fn fully_dense(&self) -> bool { - self.holes == 0 - } - - /// Set a table entry. - /// - /// The table will grow as needed to fit `idx`. - pub fn set_entry(&mut self, idx: usize, dest: Ebb) { - // Resize table to fit `idx`. - if idx >= self.table.len() { - self.holes += idx - self.table.len(); - self.table.resize(idx + 1, None.into()); - } else if self.table[idx].is_none() { - // We're filling in an existing hole. - self.holes -= 1; - } - self.table[idx] = dest.into(); - } - /// Append a table entry. pub fn push_entry(&mut self, dest: Ebb) { - self.table.push(dest.into()) - } - - /// Clear a table entry. - /// - /// The `br_table` instruction will fall through if given an index corresponding to a cleared - /// table entry. - pub fn clear_entry(&mut self, idx: usize) { - if idx < self.table.len() && self.table[idx].is_some() { - self.holes += 1; - self.table[idx] = None.into(); - } - } - - /// Get the entry for `idx`, or `None`. - pub fn get_entry(&self, idx: usize) -> Option { - self.table.get(idx).and_then(|e| e.expand()) - } - - /// Enumerate over all `(idx, dest)` pairs in the table in order. - /// - /// This returns an iterator that skips any empty slots in the table. - pub fn entries(&self) -> Entries { - Entries(self.table.iter().cloned().enumerate()) + self.table.push(dest) } /// Checks if any of the entries branch to `ebb`. pub fn branches_to(&self, ebb: Ebb) -> bool { - self.table - .iter() - .any(|target_ebb| target_ebb.expand() == Some(ebb)) + self.table.iter().any(|target_ebb| *target_ebb == ebb) + } + + /// Access the whole table as a slice. + pub fn as_slice(&self) -> &[Ebb] { + self.table.as_slice() } /// Access the whole table as a mutable slice. - pub fn as_mut_slice(&mut self) -> &mut [PackedOption] { + pub fn as_mut_slice(&mut self) -> &mut [Ebb] { self.table.as_mut_slice() } -} -/// Enumerate `(idx, dest)` pairs in order. -pub struct Entries<'a>(iter::Enumerate>>>); + /// Returns an iterator over the table. + pub fn iter(&self) -> Iter { + self.table.iter() + } -impl<'a> Iterator for Entries<'a> { - type Item = (usize, Ebb); - - fn next(&mut self) -> Option { - loop { - if let Some((idx, dest)) = self.0.next() { - if let Some(ebb) = dest.expand() { - return Some((idx, ebb)); - } - } else { - return None; - } - } + /// Returns an iterator that allows modifying each value. + pub fn iter_mut(&mut self) -> IterMut { + self.table.iter_mut() } } impl Display for JumpTableData { fn fmt(&self, fmt: &mut Formatter) -> fmt::Result { - match self.table.first().and_then(|e| e.expand()) { - None => write!(fmt, "jump_table 0")?, - Some(first) => write!(fmt, "jump_table {}", first)?, + write!(fmt, "jump_table [")?; + match self.table.first() { + None => (), + Some(first) => write!(fmt, "{}", first)?, } - - for dest in self.table.iter().skip(1).map(|e| e.expand()) { - match dest { - None => write!(fmt, ", 0")?, - Some(ebb) => write!(fmt, ", {}", ebb)?, - } + for ebb in self.table.iter().skip(1) { + write!(fmt, ", {}", ebb)?; } - Ok(()) + write!(fmt, "]") } } @@ -148,18 +86,17 @@ mod tests { use entity::EntityRef; use ir::Ebb; use std::string::ToString; - use std::vec::Vec; #[test] fn empty() { let jt = JumpTableData::new(); - assert_eq!(jt.get_entry(0), None); - assert_eq!(jt.get_entry(10), None); + assert_eq!(jt.as_slice().get(0), None); + assert_eq!(jt.as_slice().get(10), None); - assert_eq!(jt.to_string(), "jump_table 0"); + assert_eq!(jt.to_string(), "jump_table []"); - let v: Vec<(usize, Ebb)> = jt.entries().collect(); + let v = jt.as_slice(); assert_eq!(v, []); } @@ -170,16 +107,13 @@ mod tests { let mut jt = JumpTableData::new(); - jt.set_entry(0, e1); - jt.set_entry(0, e2); - jt.set_entry(10, e1); + jt.push_entry(e1); + jt.push_entry(e2); + jt.push_entry(e1); - assert_eq!( - jt.to_string(), - "jump_table ebb2, 0, 0, 0, 0, 0, 0, 0, 0, 0, ebb1" - ); + assert_eq!(jt.to_string(), "jump_table [ebb1, ebb2, ebb1]"); - let v: Vec<(usize, Ebb)> = jt.entries().collect(); - assert_eq!(v, [(0, e2), (10, e1)]); + let v = jt.as_slice(); + assert_eq!(v, [e1, e2, e1]); } } diff --git a/lib/codegen/src/legalizer/mod.rs b/lib/codegen/src/legalizer/mod.rs index 9fb3a59554..e52889c672 100644 --- a/lib/codegen/src/legalizer/mod.rs +++ b/lib/codegen/src/legalizer/mod.rs @@ -203,7 +203,6 @@ fn expand_br_table_jt( }; let table_size = func.jump_tables[table].len(); - let table_is_fully_dense = func.jump_tables[table].fully_dense(); let addr_ty = isa.pointer_type(); let entry_ty = I32; @@ -222,11 +221,6 @@ fn expand_br_table_jt( .ins() .jump_table_entry(addr_ty, arg, base_addr, entry_ty.bytes() as u8, table); - // If the table isn't fully dense, zero-check the entry. - if !table_is_fully_dense { - pos.ins().brz(entry, default_ebb, &[]); - } - let addr = pos.ins().iadd(base_addr, entry); pos.ins().indirect_jump_table_br(addr, table); @@ -260,10 +254,9 @@ fn expand_br_table_conds( pos.use_srcloc(inst); for i in 0..table_size { - if let Some(dest) = pos.func.jump_tables[table].get_entry(i) { - let t = pos.ins().icmp_imm(IntCC::Equal, arg, i as i64); - pos.ins().brnz(t, dest, &[]); - } + let dest = pos.func.jump_tables[table].as_slice()[i]; + let t = pos.ins().icmp_imm(IntCC::Equal, arg, i as i64); + pos.ins().brnz(t, dest, &[]); } // `br_table` jumps to the default destination if nothing matches diff --git a/lib/codegen/src/regalloc/coloring.rs b/lib/codegen/src/regalloc/coloring.rs index 0859bec657..3c1a11b3fb 100644 --- a/lib/codegen/src/regalloc/coloring.rs +++ b/lib/codegen/src/regalloc/coloring.rs @@ -907,8 +907,8 @@ impl<'a> Context<'a> { .cur .func .jump_tables[jt] - .entries() - .any(|(_, ebb)| lr.is_livein(ebb, ctx))) + .iter() + .any(|ebb| lr.is_livein(*ebb, ctx))) } } } diff --git a/lib/codegen/src/verifier/flags.rs b/lib/codegen/src/verifier/flags.rs index 39cdb30660..022ba5e1f3 100644 --- a/lib/codegen/src/verifier/flags.rs +++ b/lib/codegen/src/verifier/flags.rs @@ -141,8 +141,8 @@ impl<'a> FlagsVerifier<'a> { merge(&mut live_val, val, inst, errors)?; } } - for (_, dest) in self.func.jump_tables[jt].entries() { - if let Some(val) = self.livein[dest].expand() { + for dest in self.func.jump_tables[jt].iter() { + if let Some(val) = self.livein[*dest].expand() { merge(&mut live_val, val, inst, errors)?; } } diff --git a/lib/codegen/src/verifier/locations.rs b/lib/codegen/src/verifier/locations.rs index c1ed8bd1b9..4962ef916b 100644 --- a/lib/codegen/src/verifier/locations.rs +++ b/lib/codegen/src/verifier/locations.rs @@ -347,8 +347,8 @@ impl<'a> LocationVerifier<'a> { ); } } - for (_, ebb) in self.func.jump_tables[jt].entries() { - if lr.is_livein(ebb, liveness.context(&self.func.layout)) { + for ebb in self.func.jump_tables[jt].iter() { + if lr.is_livein(*ebb, liveness.context(&self.func.layout)) { return fatal!( errors, inst, diff --git a/lib/codegen/src/verifier/mod.rs b/lib/codegen/src/verifier/mod.rs index 01baa66c11..2daa1d5440 100644 --- a/lib/codegen/src/verifier/mod.rs +++ b/lib/codegen/src/verifier/mod.rs @@ -1229,8 +1229,8 @@ impl<'a> Verifier<'a> { ); } } - for (_, ebb) in self.func.jump_tables[table].entries() { - let arg_count = self.func.dfg.num_ebb_params(ebb); + for ebb in self.func.jump_tables[table].iter() { + let arg_count = self.func.dfg.num_ebb_params(*ebb); if arg_count != 0 { return nonfatal!( errors, diff --git a/lib/filetests/src/test_binemit.rs b/lib/filetests/src/test_binemit.rs index 2f0db746bd..d8dfcb617d 100644 --- a/lib/filetests/src/test_binemit.rs +++ b/lib/filetests/src/test_binemit.rs @@ -291,14 +291,9 @@ impl SubTest for TestBinEmit { for (jt, jt_data) in func.jump_tables.iter() { let jt_offset = func.jt_offsets[jt]; - for idx in 0..jt_data.len() { - match jt_data.get_entry(idx) { - Some(ebb) => { - let rel_offset: i32 = func.offsets[ebb] as i32 - jt_offset as i32; - sink.put4(rel_offset as u32) - } - None => sink.put4(0), - } + for ebb in jt_data.iter() { + let rel_offset: i32 = func.offsets[*ebb] as i32 - jt_offset as i32; + sink.put4(rel_offset as u32) } } diff --git a/lib/frontend/src/frontend.rs b/lib/frontend/src/frontend.rs index c2befee997..e637d6bf18 100644 --- a/lib/frontend/src/frontend.rs +++ b/lib/frontend/src/frontend.rs @@ -152,12 +152,11 @@ impl<'short, 'long> InstBuilderBase<'short> for FuncInstBuilder<'short, 'long> { .jump_tables .get(table) .expect("you are referencing an undeclared jump table") - .entries() - .map(|(_, ebb)| ebb) - .filter(|dest_ebb| unique.insert(*dest_ebb)) + .iter() + .filter(|&dest_ebb| unique.insert(*dest_ebb)) { self.builder.func_ctx.ssa.declare_ebb_predecessor( - dest_ebb, + *dest_ebb, self.builder.position.basic_block.unwrap(), inst, ) @@ -336,11 +335,6 @@ impl<'a> FunctionBuilder<'a> { self.func.create_jump_table(data) } - /// Inserts an entry in a previously declared jump table. - pub fn insert_jump_table_entry(&mut self, jt: JumpTable, index: usize, ebb: Ebb) { - self.func.insert_jump_table_entry(jt, index, ebb) - } - /// Creates a stack slot in the function, to be used by `stack_load`, `stack_store` and /// `stack_addr` instructions. pub fn create_stack_slot(&mut self, data: StackSlotData) -> StackSlot { diff --git a/lib/frontend/src/ssa.rs b/lib/frontend/src/ssa.rs index d13cc0531a..a78ed0b890 100644 --- a/lib/frontend/src/ssa.rs +++ b/lib/frontend/src/ssa.rs @@ -672,8 +672,8 @@ impl SSABuilder { } for old_dest in func.jump_tables[jt].as_mut_slice() { - if *old_dest == PackedOption::from(dest_ebb) { - *old_dest = PackedOption::from(middle_ebb); + if *old_dest == dest_ebb { + *old_dest = middle_ebb; } } let mut cur = FuncCursor::new(func).at_bottom(middle_ebb); @@ -1005,7 +1005,7 @@ mod tests { // Here is the pseudo-program we want to translate: // // function %f { - // jt = jump_table ebb2, 0, ebb1 + // jt = jump_table [ebb2, ebb1] // ebb0: // x = 1; // br_table x, ebb2, jt @@ -1039,9 +1039,9 @@ mod tests { }; ssa.def_var(x_var, x1, block0); - // jt = jump_table ebb2, 0, ebb1 + // jt = jump_table [ebb2, ebb1] jump_table.push_entry(ebb2); - jump_table.set_entry(2, ebb1); + jump_table.push_entry(ebb1); let jt = func.create_jump_table(jump_table); // ebb0: diff --git a/lib/frontend/src/switch.rs b/lib/frontend/src/switch.rs index 7e8db1183d..f01a9f620b 100644 --- a/lib/frontend/src/switch.rs +++ b/lib/frontend/src/switch.rs @@ -244,7 +244,7 @@ mod tests { let func = setup!(0, [0, 1,]); assert_eq!( func, - " jt0 = jump_table ebb1, ebb2 + " jt0 = jump_table [ebb1, ebb2] ebb0: v0 = iconst.i8 0 @@ -280,8 +280,8 @@ ebb3: let func = setup!(0, [0, 1, 5, 7, 10, 11, 12,]); assert_eq!( func, - " jt0 = jump_table ebb1, ebb2 - jt1 = jump_table ebb5, ebb6, ebb7 + " jt0 = jump_table [ebb1, ebb2] + jt1 = jump_table [ebb5, ebb6, ebb7] ebb0: v0 = iconst.i8 0 diff --git a/lib/reader/src/parser.rs b/lib/reader/src/parser.rs index 8809774047..4840387e5a 100644 --- a/lib/reader/src/parser.rs +++ b/lib/reader/src/parser.rs @@ -1494,48 +1494,48 @@ impl<'a> Parser<'a> { // Parse a jump table decl. // - // jump-table-decl ::= * JumpTable(jt) "=" "jump_table" jt-entry {"," jt-entry} + // jump-table-decl ::= * JumpTable(jt) "=" "jump_table" "[" jt-entry {"," jt-entry} "]" fn parse_jump_table_decl(&mut self) -> ParseResult<(JumpTable, JumpTableData)> { let jt = self.match_jt()?; self.match_token(Token::Equal, "expected '=' in jump_table decl")?; self.match_identifier("jump_table", "expected 'jump_table'")?; + self.match_token(Token::LBracket, "expected '[' before jump table contents")?; let mut data = JumpTableData::new(); - // jump-table-decl ::= JumpTable(jt) "=" "jump_table" * jt-entry {"," jt-entry} - for idx in 0_usize.. { - if let Some(dest) = self.parse_jump_table_entry()? { - data.set_entry(idx, dest); - } - if !self.optional(Token::Comma) { - // Collect any trailing comments. - self.token(); - self.claim_gathered_comments(jt); - - return Ok((jt, data)); - } - } - - err!(self.loc, "jump_table too long") - } - - // jt-entry ::= * Ebb(dest) | "0" - fn parse_jump_table_entry(&mut self) -> ParseResult> { + // jump-table-decl ::= JumpTable(jt) "=" "jump_table" "[" * Ebb(dest) {"," Ebb(dest)} "]" match self.token() { - Some(Token::Integer(s)) => { - if s == "0" { - self.consume(); - Ok(None) - } else { - err!(self.loc, "invalid jump_table entry '{}'", s) - } - } Some(Token::Ebb(dest)) => { self.consume(); - Ok(Some(dest)) + data.push_entry(dest); + + loop { + match self.token() { + Some(Token::Comma) => { + self.consume(); + if let Some(Token::Ebb(dest)) = self.token() { + self.consume(); + data.push_entry(dest); + } else { + return err!(self.loc, "expected jump_table entry"); + } + } + Some(Token::RBracket) => break, + _ => return err!(self.loc, "expected ']' after jump table contents"), + } + } } - _ => err!(self.loc, "expected jump_table entry"), + Some(Token::RBracket) => (), + _ => return err!(self.loc, "expected jump_table entry"), } + + self.consume(); + + // Collect any trailing comments. + self.token(); + self.claim_gathered_comments(jt); + + Ok((jt, data)) } // Parse a function body, add contents to `ctx`. @@ -2738,8 +2738,8 @@ mod tests { fn duplicate_jt() { let ParseError { location, message } = Parser::new( "function %ebbs() system_v { - jt0 = jump_table 0, 0 - jt0 = jump_table 0, 0", + jt0 = jump_table [] + jt0 = jump_table []", ).parse_function(None) .unwrap_err(); @@ -2820,7 +2820,7 @@ mod tests { function %comment() system_v { ; decl ss10 = outgoing_arg 13 ; stackslot. ; Still stackslot. - jt10 = jump_table ebb0 + jt10 = jump_table [ebb0] ; Jumptable ebb0: ; Basic block trap user42; Instruction diff --git a/lib/reader/src/sourcemap.rs b/lib/reader/src/sourcemap.rs index ca653d21b5..7e4da9de80 100644 --- a/lib/reader/src/sourcemap.rs +++ b/lib/reader/src/sourcemap.rs @@ -218,7 +218,7 @@ mod tests { let tf = parse_test( "function %detail() { ss10 = incoming_arg 13 - jt10 = jump_table ebb0 + jt10 = jump_table [ebb0] ebb0(v4: i32, v7: i32): v10 = iadd v4, v7 }",