Use PackedOption<Value> instead of NO_VALUE.

- Remove NO_VALUE and ExpandedValue::None.
- Remove the Default implelmentation for Value.
- InstructionData::second_result() returns an Option<Value>.
- InstructionData::second_result() returns a reference to the packed
  option.

lib/cretonne/meta/gen_instr.py

@@ -112,7 +112,8 @@ def gen_instruction_data_impl(fmt):
     - `pub fn opcode(&self) -> Opcode`
     - `pub fn first_type(&self) -> Type`
     - `pub fn second_result(&self) -> Option<Value>`
-    - `pub fn second_result_mut<'a>(&'a mut self) -> Option<&'a mut Value>`
+    - `pub fn second_result_mut<'a>(&'a mut self)
+         -> Option<&'a mut PackedOption<Value>>`
     - `pub fn arguments(&self) -> (&[Value], &[Value])`
     """
 
@@ -157,7 +158,7 @@ def gen_instruction_data_impl(fmt):
                         fmt.line(
                                 'InstructionData::' + f.name +
                                 ' { second_result, .. }' +
-                                ' => Some(second_result),')
+                                ' => second_result.into(),')
                     else:
                         # Single or no results.
                         fmt.line(
@@ -167,7 +168,7 @@ def gen_instruction_data_impl(fmt):
         fmt.doc_comment('Mutable reference to second result value, if any.')
         with fmt.indented(
                 "pub fn second_result_mut<'a>(&'a mut self)" +
-                " -> Option<&'a mut Value> {", '}'):
+                " -> Option<&'a mut PackedOption<Value>> {", '}'):
             with fmt.indented('match *self {', '}'):
                 for f in InstructionFormat.all_formats:
                     if f.multiple_results:
@@ -489,7 +490,7 @@ def gen_format_constructor(iform, fmt):
             fmt.line('opcode: opcode,')
             fmt.line('ty: {},'.format(result_type))
             if iform.multiple_results:
-                fmt.line('second_result: Value::default(),')
+                fmt.line('second_result: None.into(),')
             if iform.boxed_storage:
                 with fmt.indented(
                         'data: Box::new(instructions::{}Data {{'

lib/cretonne/src/ir/builder.rs

@@ -132,9 +132,9 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> {
     fn simple_instruction(self, data: InstructionData) -> (Inst, &'f mut DataFlowGraph) {
         // The replacement instruction cannot generate multiple results, so verify that the old
         // instruction's secondary results have been detached.
-        let old_second_value = self.dfg[self.inst].second_result().unwrap_or_default();
+        let old_second_value = self.dfg[self.inst].second_result();
         assert_eq!(old_second_value,
-                   Value::default(),
+                   None,
                    "Secondary result values {:?} would be left dangling by replacing {} with {}",
                    self.dfg.inst_results(self.inst).collect::<Vec<_>>(),
                    self.dfg[self.inst].opcode(),
@@ -150,12 +150,12 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> {
                            ctrl_typevar: Type)
                            -> (Inst, &'f mut DataFlowGraph) {
         // If the old instruction still has secondary results attached, we'll keep them.
-        let old_second_value = self.dfg[self.inst].second_result().unwrap_or_default();
+        let old_second_value = self.dfg[self.inst].second_result();
 
         // Splat the new instruction on top of the old one.
         self.dfg[self.inst] = data;
 
-        if old_second_value == Value::default() {
+        if old_second_value.is_none() {
             // The old secondary values were either detached or non-existent.
             // Construct new ones and set the first result type too.
             self.dfg.make_inst_results(self.inst, ctrl_typevar);
@@ -163,7 +163,7 @@ impl<'f> InstBuilderBase<'f> for ReplaceBuilder<'f> {
             // Reattach the old secondary values.
             if let Some(val_ref) = self.dfg[self.inst].second_result_mut() {
                 // Don't check types here. Leave that to the verifier.
-                *val_ref = old_second_value;
+                *val_ref = old_second_value.into();
             } else {
                 // Actually, this instruction format should have called `simple_instruction()`, but
                 // we don't have a rule against calling `complex_instruction()` even when it is

lib/cretonne/src/ir/dfg.rs

@@ -1,14 +1,14 @@
 //! Data flow graph tracking Instructions, Values, and EBBs.
 
 use ir::{Ebb, Inst, Value, Type, SigRef, Signature, FuncRef};
-use ir::entities::{NO_VALUE, ExpandedValue};
+use ir::entities::ExpandedValue;
 use ir::instructions::{Opcode, InstructionData, CallInfo};
 use ir::extfunc::ExtFuncData;
 use entity_map::{EntityMap, PrimaryEntityData};
 use ir::builder::{InsertBuilder, ReplaceBuilder};
 use ir::layout::Cursor;
+use packed_option::PackedOption;
 
-use std::mem;
 use std::ops::{Index, IndexMut};
 use std::u16;
 
@@ -103,7 +103,6 @@ impl DataFlowGraph {
                     ValueData::Alias { ty, .. } => ty,
                 }
             }
-            None => panic!("NO_VALUE has no type"),
         }
     }
 
@@ -126,7 +125,6 @@ impl DataFlowGraph {
                     }
                 }
             }
-            None => panic!("NO_VALUE has no def"),
         }
     }
 
@@ -237,7 +235,7 @@ enum ValueData {
         ty: Type,
         num: u16, // Result number starting from 0.
         inst: Inst,
-        next: Value, // Next result defined by `def`.
+        next: PackedOption<Value>, // Next result defined by `def`.
     },
 
     // Value is an EBB argument.
@@ -245,7 +243,7 @@ enum ValueData {
         ty: Type,
         num: u16, // Argument number, starting from 0.
         ebb: Ebb,
-        next: Value, // Next argument to `ebb`.
+        next: PackedOption<Value>, // Next argument to `ebb`.
     },
 
     // Value is an alias of another value.
@@ -262,31 +260,30 @@ enum ValueData {
 /// A value iterator borrows a `DataFlowGraph` reference.
 pub struct Values<'a> {
     dfg: &'a DataFlowGraph,
-    cur: Value,
+    cur: Option<Value>,
 }
 
 impl<'a> Iterator for Values<'a> {
     type Item = Value;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let prev = self.cur;
+        let rval = self.cur;
-
+        if let Some(prev) = rval {
-        // Advance self.cur to the next value, or NO_VALUE.
+            // Advance self.cur to the next value, or `None`.
-        self.cur = match prev.expand() {
+            self.cur = match prev.expand() {
-            ExpandedValue::Direct(inst) => self.dfg.insts[inst].second_result().unwrap_or_default(),
+                ExpandedValue::Direct(inst) => self.dfg.insts[inst].second_result(),
-            ExpandedValue::Table(index) => {
+                ExpandedValue::Table(index) => {
-                match self.dfg.extended_values[index] {
+                    match self.dfg.extended_values[index] {
-                    ValueData::Inst { next, .. } => next,
+                        ValueData::Inst { next, .. } => next.into(),
-                    ValueData::Arg { next, .. } => next,
+                        ValueData::Arg { next, .. } => next.into(),
-                    ValueData::Alias { .. } => {
+                        ValueData::Alias { .. } => {
-                        panic!("Alias value {} appeared in value list", prev)
+                            panic!("Alias value {} appeared in value list", prev)
+                        }
                     }
                 }
-            }
+            };
-            ExpandedValue::None => return None,
+        }
-        };
+        rval
-
-        Some(prev)
     }
 }
 
@@ -334,7 +331,7 @@ impl DataFlowGraph {
         // causes additional result values to be numbered backwards which is not the aestetic
         // choice, but since it is only visible in extremely rare instructions with 3+ results,
         // we don't care).
-        let mut head = NO_VALUE;
+        let mut head = None;
         let mut first_type = None;
         let mut rev_num = 1;
 
@@ -346,37 +343,37 @@ impl DataFlowGraph {
 
             for res_idx in (0..var_results).rev() {
                 if let Some(ty) = first_type {
-                    head = self.make_value(ValueData::Inst {
+                    head = Some(self.make_value(ValueData::Inst {
                         ty: ty,
                         num: (total_results - rev_num) as u16,
                         inst: inst,
-                        next: head,
+                        next: head.into(),
-                    });
+                    }));
                     rev_num += 1;
                 }
                 first_type = Some(self.signatures[sig].return_types[res_idx].value_type);
             }
         }
 
-        // Then the fixed results whic will appear at the front of the list.
+        // Then the fixed results which will appear at the front of the list.
         for res_idx in (0..fixed_results).rev() {
             if let Some(ty) = first_type {
-                head = self.make_value(ValueData::Inst {
+                head = Some(self.make_value(ValueData::Inst {
                     ty: ty,
                     num: (total_results - rev_num) as u16,
                     inst: inst,
-                    next: head,
+                    next: head.into(),
-                });
+                }));
                 rev_num += 1;
             }
             first_type = Some(constraints.result_type(res_idx, ctrl_typevar));
         }
 
         // Update the second_result pointer in `inst`.
-        if head != NO_VALUE {
+        if head.is_some() {
             *self.insts[inst]
                 .second_result_mut()
-                .expect("instruction format doesn't allow multiple results") = head;
+                .expect("instruction format doesn't allow multiple results") = head.into();
         }
         *self.insts[inst].first_type_mut() = first_type.unwrap_or_default();
 
@@ -403,8 +400,7 @@ impl DataFlowGraph {
     /// Use this method to detach secondary values before using `replace(inst)` to provide an
     /// alternate instruction for computing the primary result value.
     pub fn detach_secondary_results(&mut self, inst: Inst) -> Values {
-        let second_result =
+        let second_result = self[inst].second_result_mut().and_then(|r| r.take());
-            self[inst].second_result_mut().map(|r| mem::replace(r, NO_VALUE)).unwrap_or_default();
         Values {
             dfg: self,
             cur: second_result,
@@ -423,9 +419,9 @@ impl DataFlowGraph {
         Values {
             dfg: self,
             cur: if self.insts[inst].first_type().is_void() {
-                NO_VALUE
+                None
             } else {
-                Value::new_direct(inst)
+                Some(Value::new_direct(inst))
             },
         }
     }
@@ -494,17 +490,16 @@ impl DataFlowGraph {
 
     /// Get the number of arguments on `ebb`.
     pub fn num_ebb_args(&self, ebb: Ebb) -> usize {
-        let last_arg = self.ebbs[ebb].last_arg;
+        match self.ebbs[ebb].last_arg.expand() {
-        match last_arg.expand() {
+            None => 0,
-            ExpandedValue::None => 0,
+            Some(last_arg) => {
-            ExpandedValue::Table(idx) => {
+                if let ExpandedValue::Table(idx) = last_arg.expand() {
-                if let ValueData::Arg { num, .. } = self.extended_values[idx] {
+                    if let ValueData::Arg { num, .. } = self.extended_values[idx] {
-                    num as usize + 1
+                        return num as usize + 1;
-                } else {
+                    }
-                    panic!("inconsistent value table entry for EBB arg");
                 }
+                panic!("inconsistent value table entry for EBB arg");
             }
-            ExpandedValue::Direct(_) => panic!("inconsistent value table entry for EBB arg"),
         }
     }
 
@@ -516,25 +511,30 @@ impl DataFlowGraph {
             ty: ty,
             ebb: ebb,
             num: num_args as u16,
-            next: NO_VALUE,
+            next: None.into(),
         });
-        let last_arg = self.ebbs[ebb].last_arg;
+        match self.ebbs[ebb].last_arg.expand() {
-        match last_arg.expand() {
+            // If last_arg is `None`, we're adding the first EBB argument.
-            // If last_arg is NO_VALUE, we're adding the first EBB argument.
+            None => {
-            ExpandedValue::None => {
+                self.ebbs[ebb].first_arg = val.into();
-                self.ebbs[ebb].first_arg = val;
             }
-            // Append to linked list of arguments.
+            Some(last_arg) => {
-            ExpandedValue::Table(idx) => {
+                match last_arg.expand() {
-                if let ValueData::Arg { ref mut next, .. } = self.extended_values[idx] {
+                    // Append to linked list of arguments.
-                    *next = val;
+                    ExpandedValue::Table(idx) => {
-                } else {
+                        if let ValueData::Arg { ref mut next, .. } = self.extended_values[idx] {
-                    panic!("inconsistent value table entry for EBB arg");
+                            *next = val.into();
+                        } else {
+                            panic!("inconsistent value table entry for EBB arg");
+                        }
+                    }
+                    ExpandedValue::Direct(_) => {
+                        panic!("inconsistent value table entry for EBB arg")
+                    }
                 }
             }
-            ExpandedValue::Direct(_) => panic!("inconsistent value table entry for EBB arg"),
+        }
-        };
+        self.ebbs[ebb].last_arg = val.into();
-        self.ebbs[ebb].last_arg = val;
         val
     }
 
@@ -542,7 +542,7 @@ impl DataFlowGraph {
     pub fn ebb_args(&self, ebb: Ebb) -> Values {
         Values {
             dfg: self,
-            cur: self.ebbs[ebb].first_arg,
+            cur: self.ebbs[ebb].first_arg.into(),
         }
     }
 }
@@ -554,21 +554,21 @@ impl DataFlowGraph {
 // match the function arguments.
 #[derive(Clone)]
 struct EbbData {
-    // First argument to this EBB, or `NO_VALUE` if the block has no arguments.
+    // First argument to this EBB, or `None` if the block has no arguments.
     //
     // The arguments are all ValueData::Argument entries that form a linked list from `first_arg`
     // to `last_arg`.
-    first_arg: Value,
+    first_arg: PackedOption<Value>,
 
-    // Last argument to this EBB, or `NO_VALUE` if the block has no arguments.
+    // Last argument to this EBB, or `None` if the block has no arguments.
-    last_arg: Value,
+    last_arg: PackedOption<Value>,
 }
 
 impl EbbData {
     fn new() -> EbbData {
         EbbData {
-            first_arg: NO_VALUE,
+            first_arg: None.into(),
-            last_arg: NO_VALUE,
+            last_arg: None.into(),
         }
     }
 }

lib/cretonne/src/ir/entities.rs

@@ -100,9 +100,6 @@ pub enum ExpandedValue {
 
     /// This value is described in the extended value table.
     Table(usize),
-
-    /// This is NO_VALUE.
-    None,
 }
 
 impl Value {
@@ -129,6 +126,7 @@ impl Value {
             None
         }
     }
+
     /// Create a `Direct` value corresponding to the first value produced by `i`.
     pub fn new_direct(i: Inst) -> Value {
         let encoding = i.index() * 2;
@@ -148,9 +146,6 @@ impl Value {
     /// Expand the internal representation into something useful.
     pub fn expand(&self) -> ExpandedValue {
         use self::ExpandedValue::*;
-        if *self == NO_VALUE {
-            return None;
-        }
         let index = (self.0 / 2) as usize;
         if self.0 % 2 == 0 {
             Direct(Inst::new(index))
@@ -180,20 +175,10 @@ impl Display for Value {
         match self.expand() {
             Direct(i) => write!(fmt, "v{}", i.0),
             Table(i) => write!(fmt, "vx{}", i),
-            None => write!(fmt, "NO_VALUE"),
         }
     }
 }
 
-/// A guaranteed invalid value reference.
-pub const NO_VALUE: Value = Value(u32::MAX);
-
-impl Default for Value {
-    fn default() -> Value {
-        NO_VALUE
-    }
-}
-
 /// An opaque reference to a stack slot.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 pub struct StackSlot(u32);

lib/cretonne/src/ir/instructions.rs

@@ -16,6 +16,7 @@ use ir::condcodes::*;
 use ir::types;
 
 use ref_slice::*;
+use packed_option::PackedOption;
 
 // Include code generated by `lib/cretonne/meta/gen_instr.py`. This file contains:
 //
@@ -126,7 +127,7 @@ pub enum InstructionData {
     UnarySplit {
         opcode: Opcode,
         ty: Type,
-        second_result: Value,
+        second_result: PackedOption<Value>,
         arg: Value,
     },
     Binary {
@@ -150,7 +151,7 @@ pub enum InstructionData {
     BinaryOverflow {
         opcode: Opcode,
         ty: Type,
-        second_result: Value,
+        second_result: PackedOption<Value>,
         args: [Value; 2],
     },
     Ternary {
@@ -161,7 +162,7 @@ pub enum InstructionData {
     TernaryOverflow {
         opcode: Opcode,
         ty: Type,
-        second_result: Value,
+        second_result: PackedOption<Value>,
         data: Box<TernaryOverflowData>,
     },
     InsertLane {
@@ -207,13 +208,13 @@ pub enum InstructionData {
     Call {
         opcode: Opcode,
         ty: Type,
-        second_result: Value,
+        second_result: PackedOption<Value>,
         data: Box<CallData>,
     },
     IndirectCall {
         opcode: Opcode,
         ty: Type,
-        second_result: Value,
+        second_result: PackedOption<Value>,
         data: Box<IndirectCallData>,
     },
     Return {

lib/cretonne/src/packed_option.rs

@@ -8,6 +8,7 @@
 //! to represent `None`.
 
 use std::fmt;
+use std::mem;
 
 /// Types that have a reserved value which can't be created any other way.
 pub trait ReservedValue: Eq {
@@ -46,6 +47,11 @@ impl<T: ReservedValue> PackedOption<T> {
     pub fn unwrap(self) -> T {
         self.expand().unwrap()
     }
+
+    /// Takes the value out of the packed option, leaving a `None` in its place.
+    pub fn take(&mut self) -> Option<T> {
+        mem::replace(self, None.into()).expand()
+    }
 }
 
 impl<T: ReservedValue> Default for PackedOption<T> {

lib/reader/src/parser.rs

@@ -13,7 +13,7 @@ use cretonne::ir::{Function, Ebb, Opcode, Value, Type, FunctionName, StackSlotDa
                    FuncRef};
 use cretonne::ir::types::VOID;
 use cretonne::ir::immediates::{Imm64, Ieee32, Ieee64};
-use cretonne::ir::entities::{AnyEntity, NO_VALUE};
+use cretonne::ir::entities::AnyEntity;
 use cretonne::ir::instructions::{InstructionFormat, InstructionData, VariableArgs,
                                  TernaryOverflowData, JumpData, BranchData, CallData,
                                  IndirectCallData, ReturnData};
@@ -1125,7 +1125,7 @@ impl<'a> Parser<'a> {
                 InstructionData::UnarySplit {
                     opcode: opcode,
                     ty: VOID,
-                    second_result: NO_VALUE,
+                    second_result: None.into(),
                     arg: try!(self.match_value("expected SSA value operand")),
                 }
             }
@@ -1168,7 +1168,7 @@ impl<'a> Parser<'a> {
                 InstructionData::BinaryOverflow {
                     opcode: opcode,
                     ty: VOID,
-                    second_result: NO_VALUE,
+                    second_result: None.into(),
                     args: [lhs, rhs],
                 }
             }
@@ -1196,7 +1196,7 @@ impl<'a> Parser<'a> {
                 InstructionData::TernaryOverflow {
                     opcode: opcode,
                     ty: VOID,
-                    second_result: NO_VALUE,
+                    second_result: None.into(),
                     data: Box::new(TernaryOverflowData { args: [lhs, rhs, cin] }),
                 }
             }
@@ -1287,7 +1287,7 @@ impl<'a> Parser<'a> {
                 InstructionData::Call {
                     opcode: opcode,
                     ty: VOID,
-                    second_result: NO_VALUE,
+                    second_result: None.into(),
                     data: Box::new(CallData {
                         func_ref: func_ref,
                         varargs: args,
@@ -1305,7 +1305,7 @@ impl<'a> Parser<'a> {
                 InstructionData::IndirectCall {
                     opcode: opcode,
                     ty: VOID,
-                    second_result: NO_VALUE,
+                    second_result: None.into(),
                     data: Box::new(IndirectCallData {
                         sig_ref: sig_ref,
                         arg: callee,