lift: Read Phi type from result instead of first operand

This crashes in case the first operand references a constant, which does not have a way to extract the type.
gfx-rs · Jan 4, 2021 · 7ff6eb9 · 7ff6eb9
1 parent 9e88752
commit 7ff6eb9
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 12 deletions.
diff --git a/rspirv/lift/mod.rs b/rspirv/lift/mod.rs
@@ -14,7 +14,7 @@ use spirv;
 use std::{borrow::Borrow, mem};
 
 /// A structure that we associate an <id> with, containing
-/// both the operation token and the resutl type.
+/// both the operation token and the result type.
 struct OpInfo {
     op: Token<ops::Op>,
     ty: Option<Token<Type>>,
@@ -130,17 +130,31 @@ impl LiftContext {
                     match inst.class.opcode {
                         spirv::Op::Line => {} // skip line decorations
                         spirv::Op::Phi => {
-                            match inst.operands[0] {
-                                dr::Operand::IdRef(id) => {
-                                    let (_, info) = context.ops.lookup(id);
-                                    arguments.push(info.ty.ok_or(InstructionError::MissingResult)?);
-                                }
-                                _ => {
-                                    return Err(ConversionError::Instruction(
-                                        InstructionError::Operand(OperandError::Missing),
-                                    ))
-                                }
-                            };
+                            let ty = context.types.lookup_token(
+                                inst.result_type.ok_or(InstructionError::MissingResult)?,
+                            );
+                            arguments.push(ty);
+
+                            // Sanity-check if all source variables are of the same type
+                            for op in inst.operands.iter().step_by(2) {
+                                match op {
+                                    dr::Operand::IdRef(id) => {
+                                        if let Some((_, info)) = context.ops.lookup_safe(*id) {
+                                            assert_eq!(Some(ty), info.ty);
+                                        } else {
+                                            // let (v, info) =
+                                            //     context.constants.lookup_safe(*id).unwrap();
+                                            // TODO: Can't convert Constant back to their lowered type yet!
+                                            // assert_eq!(Some(ty), info.ty.as_ref());
+                                        }
+                                    }
+                                    _ => {
+                                        return Err(ConversionError::Instruction(
+                                            InstructionError::Operand(OperandError::Missing),
+                                        ))
+                                    }
+                                };
+                            }
                         }
                         _ => {
                             if let Some(id) = inst.result_id {

diff --git a/rspirv/lift/storage.rs b/rspirv/lift/storage.rs
@@ -32,6 +32,11 @@ impl<T, L: Borrow<Token<T>>> LiftStorage<T, L> {
         (&self.values[*info.borrow()], info)
     }
 
+    pub(in crate::lift) fn lookup_safe(&self, id: spirv::Word) -> Option<(&T, &L)> {
+        let info = self.lookup.get(&id)?;
+        Some((&self.values[*info.borrow()], info))
+    }
+
     pub(in crate::lift) fn append(
         &mut self,
         id: spirv::Word,