Tighten up the parser's verification of value aliases.

This prevents uses of undefined values from passsing through
unnoticed, and ensures that all aliases are ultimately resolved,
regardless of where they are defined.

lib/cretonne/src/ir/dfg.rs

@@ -121,8 +121,9 @@ impl DataFlowGraph {
 
 /// Resolve value aliases.
 ///
-/// Find the original SSA value that `value` aliases.
+/// Find the original SSA value that `value` aliases, or None if an
-fn resolve_aliases(values: &PrimaryMap<Value, ValueData>, value: Value) -> Value {
+/// alias cycle is detected.
+fn maybe_resolve_aliases(values: &PrimaryMap<Value, ValueData>, value: Value) -> Option<Value> {
     let mut v = value;
 
     // Note that values may be empty here.
@@ -130,10 +131,22 @@ fn resolve_aliases(values: &PrimaryMap<Value, ValueData>, value: Value) -> Value
         if let ValueData::Alias { original, .. } = values[v] {
             v = original;
         } else {
-            return v;
+            return Some(v);
         }
     }
-    panic!("Value alias loop detected for {}", value);
+
+    None
+}
+
+/// Resolve value aliases.
+///
+/// Find the original SSA value that `value` aliases.
+fn resolve_aliases(values: &PrimaryMap<Value, ValueData>, value: Value) -> Value {
+    if let Some(v) = maybe_resolve_aliases(values, value) {
+        v
+    } else {
+        panic!("Value alias loop detected for {}", value);
+    }
 }
 
 /// Handling values.
@@ -238,6 +251,7 @@ impl DataFlowGraph {
             self.value_type(dest),
             ty
         );
+        debug_assert_ne!(ty, types::VOID);
 
         self.values[dest] = ValueData::Alias { ty, original };
     }
@@ -282,6 +296,7 @@ impl DataFlowGraph {
                 self.value_type(dest),
                 ty
             );
+            debug_assert_ne!(ty, types::VOID);
 
             self.values[dest] = ValueData::Alias { ty, original };
         }
@@ -865,8 +880,9 @@ impl DataFlowGraph {
     /// to create invalid values for index padding which may be reassigned later.
     #[cold]
     fn set_value_type_for_parser(&mut self, v: Value, t: Type) {
-        assert!(
+        assert_eq!(
-            self.value_type(v) == types::VOID,
+            self.value_type(v),
+            types::VOID,
             "this function is only for assigning types to previously invalid values"
         );
         match self.values[v] {
@@ -926,12 +942,38 @@ impl DataFlowGraph {
     /// aliases with specific values.
     #[cold]
     pub fn make_value_alias_for_parser(&mut self, src: Value, dest: Value) {
-        let ty = self.value_type(src);
+        assert_ne!(src, Value::reserved_value());
+        assert_ne!(dest, Value::reserved_value());
 
+        let ty = if self.values.is_valid(src) {
+            self.value_type(src)
+        } else {
+            // As a special case, if we can't resolve the aliasee yet, use VOID
+            // temporarily. It will be resolved later in parsing.
+            types::VOID
+        };
         let data = ValueData::Alias { ty, original: src };
         self.values[dest] = data;
     }
 
+    /// Compute the type of an alias. This is only for use in the parser.
+    /// Returns false if an alias cycle was encountered.
+    #[cold]
+    pub fn set_alias_type_for_parser(&mut self, v: Value) -> bool {
+        if let Some(resolved) = maybe_resolve_aliases(&self.values, v) {
+            let old_ty = self.value_type(v);
+            let new_ty = self.value_type(resolved);
+            if old_ty == types::VOID {
+                self.set_value_type_for_parser(v, new_ty);
+            } else {
+                assert_eq!(old_ty, new_ty);
+            }
+            true
+        } else {
+            false
+        }
+    }
+
     /// Create an invalid value, to pad the index space. This is only for use by
     /// the parser to pad out the value index space.
     #[cold]
@@ -942,6 +984,20 @@ impl DataFlowGraph {
         };
         self.make_value(data);
     }
+
+    /// Check if a value reference is valid, while being aware of aliases which
+    /// may be unresolved while parsing.
+    #[cold]
+    pub fn value_is_valid_for_parser(&self, v: Value) -> bool {
+        if !self.value_is_valid(v) {
+            return false;
+        }
+        if let ValueData::Alias { ty, .. } = self.values[v] {
+            ty != types::VOID
+        } else {
+            true
+        }
+    }
 }
 
 #[cfg(test)]

lib/reader/src/parser.rs

@@ -86,6 +86,9 @@ struct Context<'a> {
     function: Function,
     map: SourceMap,
 
+    // Aliases to resolve once value definitions are known.
+    aliases: Vec<Value>,
+
     // Reference to the unique_isa for things like parsing ISA-specific instruction encoding
     // information. This is only `Some` if exactly one set of `isa` directives were found in the
     // prologue (it is valid to have directives for multiple different ISAs, but in that case we
@@ -99,6 +102,7 @@ impl<'a> Context<'a> {
             function: f,
             map: SourceMap::new(),
             unique_isa,
+            aliases: Vec::new(),
         }
     }
 
@@ -1341,6 +1345,30 @@ impl<'a> Parser<'a> {
         while self.token() != Some(Token::RBrace) {
             self.parse_extended_basic_block(ctx)?;
         }
+
+        // Now that we've seen all defined values in the function, ensure that
+        // all references refer to a definition.
+        for ebb in &ctx.function.layout {
+            for inst in ctx.function.layout.ebb_insts(ebb) {
+                for value in ctx.function.dfg.inst_args(inst) {
+                    if !ctx.map.contains_value(*value) {
+                        return err!(
+                            ctx.map.location(AnyEntity::Inst(inst)).unwrap(),
+                            "undefined operand value {}",
+                            value
+                        );
+                    }
+                }
+            }
+        }
+
+        for alias in &ctx.aliases {
+            if !ctx.function.dfg.set_alias_type_for_parser(*alias) {
+                let loc = ctx.map.location(AnyEntity::Value(*alias)).unwrap();
+                return err!(loc, "alias cycle involving {}", alias);
+            }
+        }
+
         Ok(())
     }
 
@@ -1616,6 +1644,7 @@ impl<'a> Parser<'a> {
             results[0],
         );
         ctx.map.def_value(results[0], &self.loc)?;
+        ctx.aliases.push(results[0]);
         Ok(())
     }
 
@@ -1758,6 +1787,26 @@ impl<'a> Parser<'a> {
                     let ctrl_src_value = inst_data
                         .typevar_operand(&ctx.function.dfg.value_lists)
                         .expect("Constraints <-> Format inconsistency");
+                    if !ctx.map.contains_value(ctrl_src_value) {
+                        return err!(
+                            self.loc,
+                            "type variable required for polymorphic opcode, e.g. '{}.{}'; \
+                            can't infer from {} which is not yet defined",
+                            opcode,
+                            constraints.ctrl_typeset().unwrap().example(),
+                            ctrl_src_value
+                        );
+                    }
+                    if !ctx.function.dfg.value_is_valid_for_parser(ctrl_src_value) {
+                        return err!(
+                            self.loc,
+                            "type variable required for polymorphic opcode, e.g. '{}.{}'; \
+                            can't infer from {} which is not yet resolved",
+                            opcode,
+                            constraints.ctrl_typeset().unwrap().example(),
+                            ctrl_src_value
+                        );
+                    }
                     ctx.function.dfg.value_type(ctrl_src_value)
                 } else if constraints.is_polymorphic() {
                     // This opcode does not support type inference, so the explicit type