Removed the Opcode::NotAnOpcode variant, replaced its uses with Option<Opcode>, and used the NonZero optimization to maintain the small 1-byte size of an optional Opcode.

lib/cretonne/meta/gen_encoding.py

@@ -364,12 +364,12 @@ def emit_level2_hashtables(level2_hashtables, offt, level2_doc, fmt):
             if entry:
                 fmt.line(
                         'Level2Entry ' +
-                        '{{ opcode: Opcode::{}, offset: {:#08x} }},'
+                        '{{ opcode: Some(Opcode::{}), offset: {:#08x} }},'
                         .format(entry.inst.camel_name, entry.offset))
             else:
                 fmt.line(
                         'Level2Entry ' +
-                        '{ opcode: Opcode::NotAnOpcode, offset: 0 },')
+                        '{ opcode: None, offset: 0 },')
 
 
 def emit_level1_hashtable(cpumode, level1, offt, fmt):

lib/cretonne/meta/gen_instr.py

@@ -254,9 +254,13 @@ def gen_opcodes(groups, fmt):
     fmt.doc_comment('All instructions from all supported ISAs are present.')
     fmt.line('#[derive(Copy, Clone, PartialEq, Eq, Debug)]')
     instrs = []
+
+    # We explicitly set the discriminant of the first variant to 1, which
+    # allows us to take advantage of the NonZero optimization, meaning that
+    # wrapping enums can use the 0 discriminant instead of increasing the size
+    # if the whole type, and so SIZEOF(Option<Opcode>>) == SIZEOF(Opcode)
+    is_first_opcode = True
     with fmt.indented('pub enum Opcode {', '}'):
-        fmt.doc_comment('An invalid opcode.')
-        fmt.line('NotAnOpcode,')
         for g in groups:
             for i in g.instructions:
                 instrs.append(i)
@@ -269,7 +273,13 @@ def gen_opcodes(groups, fmt):
                                 'Type inferred from {}.'
                                 .format(i.ins[i.format.typevar_operand]))
                 # Enum variant itself.
-                fmt.line(i.camel_name + ',')
+                if is_first_opcode:
+                    fmt.doc_comment('We explicitly set this to 1 to allow the NonZero optimization,')
+                    fmt.doc_comment('meaning that SIZEOF(Option<Opcode>) == SIZEOF(Opcode)')
+                    fmt.line(i.camel_name + ' = 1,')
+                    is_first_opcode = False
+                else:
+                    fmt.line(i.camel_name + ',')
     fmt.line()
 
     with fmt.indented('impl Opcode {', '}'):
@@ -318,7 +328,6 @@ def gen_opcodes(groups, fmt):
     # Generate a private opcode_name function.
     with fmt.indented('fn opcode_name(opc: Opcode) -> &\'static str {', '}'):
         with fmt.indented('match opc {', '}'):
-            fmt.line('Opcode::NotAnOpcode => "<not an opcode>",')
             for i in instrs:
                 fmt.format('Opcode::{} => "{}",', i.camel_name, i.name)
     fmt.line()
@@ -328,13 +337,13 @@ def gen_opcodes(groups, fmt):
             instrs,
             lambda i: constant_hash.simple_hash(i.name))
     with fmt.indented(
-            'const OPCODE_HASH_TABLE: [Opcode; {}] = ['
+            'const OPCODE_HASH_TABLE: [Option<Opcode>; {}] = ['
             .format(len(hash_table)), '];'):
         for i in hash_table:
             if i is None:
-                fmt.line('Opcode::NotAnOpcode,')
+                fmt.line('None,')
             else:
-                fmt.format('Opcode::{},', i.camel_name)
+                fmt.format('Some(Opcode::{}),', i.camel_name)
     fmt.line()
     return instrs
 

lib/cretonne/src/ir/instructions.rs

@@ -43,12 +43,8 @@ impl Display for Opcode {
 
 impl Opcode {
     /// Get the instruction format for this opcode.
-    pub fn format(self) -> Option<InstructionFormat> {
+    pub fn format(self) -> InstructionFormat {
-        if self == Opcode::NotAnOpcode {
+        OPCODE_FORMAT[self as usize - 1]
-            None
-        } else {
-            Some(OPCODE_FORMAT[self as usize - 1])
-        }
     }
 
     /// Get the constraint descriptor for this opcode.
@@ -69,23 +65,21 @@ impl FromStr for Opcode {
     fn from_str(s: &str) -> Result<Opcode, &'static str> {
         use constant_hash::{Table, simple_hash, probe};
 
-        impl<'a> Table<&'a str> for [Opcode] {
+        impl<'a> Table<&'a str> for [Option<Opcode>] {
             fn len(&self) -> usize {
                 self.len()
             }
 
             fn key(&self, idx: usize) -> Option<&'a str> {
-                if self[idx] == Opcode::NotAnOpcode {
+                self[idx].map(opcode_name)
-                    None
-                } else {
-                    Some(opcode_name(self[idx]))
-                }
             }
         }
 
-        match probe::<&str, [Opcode]>(&OPCODE_HASH_TABLE, s, simple_hash(s)) {
+        match probe::<&str, [Option<Opcode>]>(&OPCODE_HASH_TABLE, s, simple_hash(s)) {
             None => Err("Unknown opcode"),
-            Some(i) => Ok(OPCODE_HASH_TABLE[i]),
+            // We unwrap here because probe() should have ensured that the entry
+            // at this index is not None.
+            Some(i) => Ok(OPCODE_HASH_TABLE[i].unwrap()),
         }
     }
 }

lib/cretonne/src/isa/enc_tables.rs

@@ -53,7 +53,7 @@ impl<OffT: Into<u32> + Copy> Table<Type> for [Level1Entry<OffT>] {
 ///
 /// Empty entries are encoded with a `NotAnOpcode` `opcode` field.
 pub struct Level2Entry<OffT: Into<u32> + Copy> {
-    pub opcode: Opcode,
+    pub opcode: Option<Opcode>,
     pub offset: OffT,
 }
 
@@ -63,12 +63,7 @@ impl<OffT: Into<u32> + Copy> Table<Opcode> for [Level2Entry<OffT>] {
     }
 
     fn key(&self, idx: usize) -> Option<Opcode> {
-        let opc = self[idx].opcode;
+        self[idx].opcode
-        if opc != Opcode::NotAnOpcode {
-            Some(opc)
-        } else {
-            None
-        }
     }
 }
 

lib/cretonne/src/verifier.rs

@@ -149,7 +149,7 @@ impl<'a> Verifier<'a> {
         let inst_data = &self.func.dfg[inst];
 
         // The instruction format matches the opcode
-        if inst_data.opcode().format() != Some(InstructionFormat::from(inst_data)) {
+        if inst_data.opcode().format() != InstructionFormat::from(inst_data) {
             return err!(inst, "instruction opcode doesn't match instruction format");
         }
 

lib/reader/src/parser.rs

@@ -1088,7 +1088,7 @@ impl<'a> Parser<'a> {
     // Parse the operands following the instruction opcode.
     // This depends on the format of the opcode.
     fn parse_inst_operands(&mut self, ctx: &Context, opcode: Opcode) -> Result<InstructionData> {
-        Ok(match opcode.format().unwrap() {
+        Ok(match opcode.format() {
             InstructionFormat::Nullary => {
                 InstructionData::Nullary {
                     opcode: opcode,