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

@@ -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
+    instruction. Entries in the table are EBB names.
-    indicates an absent entry.
 
     The EBBs listed must belong to the current function, and they can't have
     any arguments.

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

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

cranelift/filetests/parser/branch.clif

@@ -81,8 +81,8 @@ ebb1(v92: i32, v93: f32):
 ; nextln: }
 
 function %jumptable(i32) {
-    jt200 = jump_table 0, 0
+    jt200 = jump_table []
-    jt2 = jump_table 0, 0, ebb10, ebb40, ebb20, ebb30
+    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:      jt2 = jump_table [ebb10, ebb40, ebb20, ebb30]
-; check:      jt200 = jump_table 0
+; check:      jt200 = jump_table []
 ; check:  ebb10(v3: i32):
 ; nextln:     br_table v3, ebb50, jt2
 ; nextln: 

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

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

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() {
+        for ebb in jt_data.iter() {
-            match jt_data.get_entry(idx) {
+            let rel_offset: i32 = func.offsets[*ebb] as i32 - jt_offset as i32;
-                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),
-            }
-        }
     }
 }

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() {
+                    for succ in func.jump_tables[jt].iter() {
-                        self.push_if_unseen(succ);
+                        self.push_if_unseen(*succ);
                     }
                     if let Some(dest) = dest {
                         self.push_if_unseen(dest);

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() {
+                    for dest in func.jump_tables[jt].iter() {
-                        self.add_edge(ebb, inst, dest);
+                        self.add_edge(ebb, inst, *dest);
                     }
                 }
                 BranchInfo::NotABranch => {}

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 {

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::{Iter, IterMut};
-use std::slice;
 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
+/// All jump tables use 0-based indexing and densely populated.
-/// to be completely populated, though. Individual entries can be missing.
 #[derive(Clone)]
 pub struct JumpTableData {
-    // Table entries, using `None` as a placeholder for missing entries.
+    // Table entries.
-    table: Vec<PackedOption<Ebb>>,
+    table: Vec<Ebb>,
-
-    // How many `None` holes in table?
-    holes: usize,
 }
 
 impl JumpTableData {
     /// Create a new empty jump table.
     pub fn new() -> Self {
-        Self {
+        Self { table: Vec::new() }
-            table: Vec::new(),
-            holes: 0,
-        }
     }
 
     /// 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())
+        self.table.push(dest)
-    }
-
-    /// 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<Ebb> {
-        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())
     }
 
     /// Checks if any of the entries branch to `ebb`.
     pub fn branches_to(&self, ebb: Ebb) -> bool {
-        self.table
+        self.table.iter().any(|target_ebb| *target_ebb == ebb)
-            .iter()
+    }
-            .any(|target_ebb| target_ebb.expand() == Some(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<Ebb>] {
+    pub fn as_mut_slice(&mut self) -> &mut [Ebb] {
         self.table.as_mut_slice()
     }
+
+    /// Returns an iterator over the table.
+    pub fn iter(&self) -> Iter<Ebb> {
+        self.table.iter()
     }
 
-/// Enumerate `(idx, dest)` pairs in order.
+    /// Returns an iterator that allows modifying each value.
-pub struct Entries<'a>(iter::Enumerate<iter::Cloned<slice::Iter<'a, PackedOption<Ebb>>>>);
+    pub fn iter_mut(&mut self) -> IterMut<Ebb> {
-
+        self.table.iter_mut()
-impl<'a> Iterator for Entries<'a> {
-    type Item = (usize, Ebb);
-
-    fn next(&mut self) -> Option<Self::Item> {
-        loop {
-            if let Some((idx, dest)) = self.0.next() {
-                if let Some(ebb) = dest.expand() {
-                    return Some((idx, ebb));
-                }
-            } else {
-                return None;
-            }
-        }
     }
 }
 
 impl Display for JumpTableData {
     fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
-        match self.table.first().and_then(|e| e.expand()) {
+        write!(fmt, "jump_table [")?;
-            None => write!(fmt, "jump_table 0")?,
+        match self.table.first() {
-            Some(first) => write!(fmt, "jump_table {}", first)?,
+            None => (),
+            Some(first) => write!(fmt, "{}", first)?,
         }
-
+        for ebb in self.table.iter().skip(1) {
-        for dest in self.table.iter().skip(1).map(|e| e.expand()) {
+            write!(fmt, ", {}", ebb)?;
-            match dest {
-                None => write!(fmt, ", 0")?,
-                Some(ebb) => write!(fmt, ", {}", ebb)?,
         }
-        }
+        write!(fmt, "]")
-        Ok(())
     }
 }
 
@@ -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.as_slice().get(0), None);
-        assert_eq!(jt.get_entry(10), 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.push_entry(e1);
-        jt.set_entry(0, e2);
+        jt.push_entry(e2);
-        jt.set_entry(10, e1);
+        jt.push_entry(e1);
 
-        assert_eq!(
+        assert_eq!(jt.to_string(), "jump_table [ebb1, ebb2, ebb1]");
-            jt.to_string(),
-            "jump_table ebb2, 0, 0, 0, 0, 0, 0, 0, 0, 0, ebb1"
-        );
 
-        let v: Vec<(usize, Ebb)> = jt.entries().collect();
+        let v = jt.as_slice();
-        assert_eq!(v, [(0, e2), (10, e1)]);
+        assert_eq!(v, [e1, e2, e1]);
     }
 }

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,11 +254,10 @@ 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 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
     pos.ins().jump(default_ebb, &[]);

lib/codegen/src/regalloc/coloring.rs

@@ -907,8 +907,8 @@ impl<'a> Context<'a> {
                         .cur
                         .func
                         .jump_tables[jt]
-                        .entries()
+                        .iter()
-                        .any(|(_, ebb)| lr.is_livein(ebb, ctx)))
+                        .any(|ebb| lr.is_livein(*ebb, ctx)))
             }
         }
     }

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() {
+                    for dest in self.func.jump_tables[jt].iter() {
-                        if let Some(val) = self.livein[dest].expand() {
+                        if let Some(val) = self.livein[*dest].expand() {
                             merge(&mut live_val, val, inst, errors)?;
                         }
                     }

lib/codegen/src/verifier/locations.rs

@@ -347,8 +347,8 @@ impl<'a> LocationVerifier<'a> {
                             );
                         }
                     }
-                    for (_, ebb) in self.func.jump_tables[jt].entries() {
+                    for ebb in self.func.jump_tables[jt].iter() {
-                        if lr.is_livein(ebb, liveness.context(&self.func.layout)) {
+                        if lr.is_livein(*ebb, liveness.context(&self.func.layout)) {
                             return fatal!(
                                 errors,
                                 inst,

lib/codegen/src/verifier/mod.rs

@@ -1229,8 +1229,8 @@ impl<'a> Verifier<'a> {
                         );
                     }
                 }
-                for (_, ebb) in self.func.jump_tables[table].entries() {
+                for ebb in self.func.jump_tables[table].iter() {
-                    let arg_count = self.func.dfg.num_ebb_params(ebb);
+                    let arg_count = self.func.dfg.num_ebb_params(*ebb);
                     if arg_count != 0 {
                         return nonfatal!(
                             errors,

lib/filetests/src/test_binemit.rs

@@ -291,15 +291,10 @@ 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() {
+            for ebb in jt_data.iter() {
-                match jt_data.get_entry(idx) {
+                let rel_offset: i32 = func.offsets[*ebb] as i32 - jt_offset as i32;
-                    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),
-                }
-            }
         }
 
         if sink.offset != code_size {

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()
+                            .iter()
-                            .map(|(_, ebb)| ebb)
+                            .filter(|&dest_ebb| unique.insert(*dest_ebb))
-                            .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 {

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) {
+                    if *old_dest == dest_ebb {
-                        *old_dest = PackedOption::from(middle_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:

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
+            "    jt0 = jump_table [ebb1, ebb2]
-    jt1 = jump_table ebb5, ebb6, ebb7
+    jt1 = jump_table [ebb5, ebb6, ebb7]
 
 ebb0:
     v0 = iconst.i8 0

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}
+        // jump-table-decl ::= JumpTable(jt) "=" "jump_table" "[" * Ebb(dest) {"," Ebb(dest)} "]"
-        for idx in 0_usize.. {
+        match self.token() {
-            if let Some(dest) = self.parse_jump_table_entry()? {
+            Some(Token::Ebb(dest)) => {
-                data.set_entry(idx, dest);
+                self.consume();
+                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");
                             }
-            if !self.optional(Token::Comma) {
+                        }
+                        Some(Token::RBracket) => break,
+                        _ => return err!(self.loc, "expected ']' after jump table contents"),
+                    }
+                }
+            }
+            Some(Token::RBracket) => (),
+            _ => return err!(self.loc, "expected jump_table entry"),
+        }
+
+        self.consume();
+
         // Collect any trailing comments.
         self.token();
         self.claim_gathered_comments(jt);
 
-                return Ok((jt, data));
+        Ok((jt, data))
-            }
-        }
-
-        err!(self.loc, "jump_table too long")
-    }
-
-    // jt-entry ::= * Ebb(dest) | "0"
-    fn parse_jump_table_entry(&mut self) -> ParseResult<Option<Ebb>> {
-        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))
-            }
-            _ => err!(self.loc, "expected jump_table entry"),
-        }
     }
 
     // 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 []
-                jt0 = jump_table 0, 0",
+                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

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
                              }",