Disaggregate by-value OpTypeStruct/OpTypeArray inputs/outputs.

README.md

@@ -135,7 +135,7 @@ fn main() -> @location(0) i32 {
 #[spv.Decoration.Location(Location: 0)]
 global_var GV0 in spv.StorageClass.Output: s32
 
-func F0() -> spv.OpTypeVoid {
+func F0() {
   (_: s32, _: s32, v0: s32) = loop(v1: s32 <- 1s32, v2: s32 <- 1s32, _: s32 <- undef: s32) {
     v3 = s.lt(v2, 10s32): bool
     (v4: s32, v5: s32) = if v3 {

examples/spv-lower-link-qptr-lift.rs

@@ -83,7 +83,7 @@ fn main() -> std::io::Result<()> {
             let layout_config = &spirt::mem::LayoutConfig {
                 abstract_bool_size_align: (1, 1),
                 logical_ptr_size_align: (4, 4),
-                ..spirt::mem::LayoutConfig::VULKAN_SCALAR_LAYOUT
+                ..spirt::mem::LayoutConfig::VULKAN_SCALAR_LAYOUT_LE
             };
 
             eprint_duration(|| {

src/cf/structurize.rs

@@ -600,15 +600,7 @@ impl<'a> Structurizer<'a> {
             const_true,
             const_false,
 
-            func_ret_types: {
-                let is_void = match &cx[func_decl.ret_type].kind {
-                    TypeKind::SpvInst { spv_inst, .. } => {
-                        spv_inst.opcode == crate::spv::spec::Spec::get().well_known.OpTypeVoid
-                    }
-                    _ => false,
-                };
-                if is_void { &[][..] } else { std::slice::from_ref(&func_decl.ret_type) }
-            },
+            func_ret_types: &func_decl.ret_types,
             func_def_body,
 
             loop_header_to_exit_targets,

src/cf/unstructured.rs

@@ -61,7 +61,8 @@ pub struct ControlFlowGraph {
 pub enum ControlTarget {
     Region(Region),
 
-    /// Leave the current function (returning `target_inputs`, if any).
+    /// Leave the current function, returning some number of [`Value`]s, as per
+    /// the function's signature (`ret_types` in [`FuncDecl`](crate::FuncDecl)).
     //
     // FIXME(eddyb) now that this is used through `NodeKind::ThunkBind`,
     // it should probably be more like `break` or some kind of "leave scope".

src/lib.rs

@@ -572,10 +572,12 @@ pub enum TypeKind {
     // TODO(eddyb) reconsider name? add signature? etc.
     Thunk,
 
+    // FIXME(eddyb) consider wrapping all of these in an `Rc` like `ConstKind`.
     SpvInst {
         spv_inst: spv::Inst,
         // FIXME(eddyb) find a better name.
         type_and_const_inputs: SmallVec<[TypeOrConst; 2]>,
+        value_lowering: spv::ValueLowering,
     },
 
     /// The type of a [`ConstKind::SpvStringLiteralForExtInst`] constant, i.e.
@@ -620,10 +622,12 @@ impl Type {
     }
 }
 
-/// Interned handle for a [`ConstDef`](crate::ConstDef) (a constant value).
+/// Interned handle for a [`ConstDef`](crate::ConstDef) (a constant [`Value`](crate::Value)).
 pub use context::Const;
 
-/// Definition for a [`Const`]: a constant value.
+/// Definition for a [`Const`]: a constant [`Value`].
+///
+/// See [`Value`] docs for limitations on the types of values, including [`Const`]s.
 //
 // FIXME(eddyb) maybe special-case some basic consts like integer literals.
 #[derive(PartialEq, Eq, Hash)]
@@ -782,10 +786,31 @@ pub enum AddrSpace {
 }
 
 /// The body of a [`GlobalVar`] definition.
+//
+// FIXME(eddyb) make "interface variables" go through imports, not definitions.
 #[derive(Clone)]
 pub struct GlobalVarDefBody {
-    /// If `Some`, the global variable will start out with the specified value.
-    pub initializer: Option<Const>,
+    pub initializer: Option<GlobalVarInit>,
+}
+
+/// Initial contents for a [`GlobalVar`] definition.
+//
+// FIXME(eddyb) add special cases for for undef/zeroed/etc.
+// FIXME(eddyb) consider renaming this to `ConstData` or `ConstBlob`?
+#[derive(Clone)]
+pub enum GlobalVarInit {
+    /// Single valid (constant) value (see [`Value`] docs for valid types).
+    //
+    // FIXME(eddyb) does this need to be its own case at all?
+    Direct(Const),
+
+    /// SPIR-V "aggregate" (`OpTypeStruct`/`OpTypeArray`), represented as its
+    /// non-aggregate leaves (i.e. it's disaggregated, as per [`Value`] docs).
+    SpvAggregate { ty: Type, leaves: SmallVec<[Const; 4]> },
+
+    /// Explicitly laid out constant data, using [`mem::const_data::ConstData`]
+    /// to efficiently mix concrete bytes with symbolic ([`Const`]) values.
+    Data(mem::const_data::ConstData<Const>),
 }
 
 /// Entity handle for a [`FuncDecl`](crate::FuncDecl) (a function).
@@ -796,7 +821,7 @@ pub use context::Func;
 pub struct FuncDecl {
     pub attrs: AttrSet,
 
-    pub ret_type: Type,
+    pub ret_types: SmallVec<[Type; 2]>,
 
     pub params: SmallVec<[FuncParam; 2]>,
 
@@ -964,7 +989,19 @@ pub struct NodeDef {
     ///   child [`Region`]:
     ///   * when this is a `Select`: the case that was chosen
     ///   * when this is a `Loop`: the last iteration of the body
-    // TODO(eddyb) include former `DataInst`s in above docs.
+    ///
+    //
+    // FIXME(eddyb) recombine with these `DataInstDef` docs:
+    //
+    /// Types (and attributes) for all the outputs of this instruction.
+    ///
+    /// That is, `vars[outputs[i]].ty` is the type of the [`VarKind::DataInstOutput`]
+    /// with `output_idx == i` (see also [`Value`] documentation).
+    ///
+    /// Most instructions have `0` or `1` outputs, with the notable exception
+    /// of SPIR-V instructions which originally produced SPIR-V "aggregates"
+    /// (`OpTypeStruct`/`OpTypeArray`) before [`spv::lower`] decomposed them
+    /// (in the general case, [`spv::InstLowering`] tracks original types).
     pub outputs: SmallVec<[Var; 2]>,
 }
 
@@ -1030,10 +1067,11 @@ pub enum NodeKind {
     ThunkBind(cf::unstructured::ControlTarget),
 
     // FIXME(eddyb) should this have `#[from]`?
-    SpvInst(spv::Inst),
+    SpvInst(spv::Inst, spv::InstLowering),
     SpvExtInst {
         ext_set: InternedStr,
         inst: u32,
+        lowering: spv::InstLowering,
     },
 }
 
@@ -1097,6 +1135,32 @@ pub enum VarKind {
     NodeOutput { node: Node, output_idx: u32 },
 }
 
+/// Use of a value, either constant or defined earlier in the same function.
+///
+/// Each `Value` can only have one of these types:
+/// * [`scalar`] (`bool`, integer, and floating-point), i.e. [`TypeKind::Scalar`]
+/// * vectors (small array of [`scalar`]s)
+///   * these are *not* traditional SIMD vectors, but more a form of "compression"
+///     (i.e. vector ops often applying the equivalent scalar op per-component),
+///     and sometimes also mandated by specs (e.g. some Vulkan `BuiltIn` types)
+/// * matrices (small array of vectors)
+///   * less fundamental than vectors, may be treated like arrays in the future
+/// * pointers and by-value (but still opaque) resource handles
+///   * SPIR-V has both opaque resource handles that behave much like pointers,
+///     even physical ones (e.g. ray-tracing `OpTypeAccelerationStructureKHR`s),
+///     and others that are only loaded from memory just before using them as
+///     operands (e.g. images/samplers), and such mismatches in indirection may
+///     result in SPIR-T making further distinctions here in the future
+///
+/// Notably, "aggregate" types (SPIR-V `OpTypeStruct`/`OpTypeArray`) are excluded,
+/// so they have to be (recursively) disaggregated into their constituents, and
+/// passed around as separate `Value`s (see also [`DataInstDef`] docs).
+/// * SPIR-V inherited "by-value aggregates" from LLVM, which supports them under
+///   the name "FCA" ("first-class aggregates"), but other IRs (and LLVM passes)
+///   avoid them because of their (negative) impact on analyses and transforms,
+///   with their main vestigial purpose being to encode multiple return values
+///   from functions, which can be done more directly in other IRs (and SPIR-T)
+//
 // FIXME(eddyb) add a `context::PackedEither` using the sign of s/u32/i32/
 // interned/entity, and use it to be more compact than `Either<Const, Var>`.
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]

src/mem/analyze.rs

@@ -923,7 +923,7 @@ impl<'a> GatherAccesses<'a> {
             let node_def = func_def_body.at(node).def();
 
             // FIXME(eddyb) consider avoiding this collection step.
-            let per_output_accesses = small_vec_from_position_value_pairs::<_, 1>(
+            let mut per_output_accesses = small_vec_from_position_value_pairs::<_, 1>(
                 node_def
                     .outputs
                     .iter()
@@ -1053,13 +1053,19 @@ impl<'a> GatherAccesses<'a> {
                 | DataInstKind::Mem(_)
                 | DataInstKind::QPtr(_)
                 | DataInstKind::ThunkBind(_)
-                | DataInstKind::SpvInst(_)
+                | DataInstKind::SpvInst(..)
                 | DataInstKind::SpvExtInst { .. } => {}
             }
 
-            // HACK(eddyb) multi-output instructions don't exist pre-disaggregate.
-            assert!(per_output_accesses.len() <= 1);
-            let output_accesses = per_output_accesses.into_iter().next().flatten();
+            // HACK(eddyb) this may be a bit wasteful, but it avoids
+            // complicating acessing `per_output_accesses` below, and
+            // most instructions should only have at most two outputs.
+            {
+                let expected = node_def.outputs.len();
+                if per_output_accesses.len() < expected {
+                    per_output_accesses.extend((per_output_accesses.len()..expected).map(|_| None));
+                }
+            }
 
             // FIXME(eddyb) merge with `match &node_def.kind` above.
             let data_inst_def = node_def;
@@ -1095,10 +1101,12 @@ impl<'a> GatherAccesses<'a> {
                     // with the inherent size/align (given by `_mem_layout`)?
                 }
                 DataInstKind::QPtr(QPtrOp::HandleArrayIndex) => {
+                    assert_eq!(per_output_accesses.len(), 1);
                     generate_accesses(
                         self,
                         data_inst_def.inputs[0],
-                        output_accesses
+                        per_output_accesses[0]
+                            .take()
                             .unwrap_or_else(|| {
                                 Err(AnalysisError(Diag::bug([
                                     "HandleArrayIndex: unknown element".into()
@@ -1113,11 +1121,14 @@ impl<'a> GatherAccesses<'a> {
                     );
                 }
                 DataInstKind::QPtr(QPtrOp::BufferData) => {
+                    assert_eq!(per_output_accesses.len(), 1);
                     generate_accesses(
                         self,
                         data_inst_def.inputs[0],
-                        output_accesses.unwrap_or(Ok(MemAccesses::Data(DataHapp::DEAD))).and_then(
-                            |accesses| {
+                        per_output_accesses[0]
+                            .take()
+                            .unwrap_or(Ok(MemAccesses::Data(DataHapp::DEAD)))
+                            .and_then(|accesses| {
                                 let happ = match accesses {
                                     MemAccesses::Handles(_) => {
                                         return Err(AnalysisError(Diag::bug([
@@ -1130,8 +1141,7 @@ impl<'a> GatherAccesses<'a> {
                                     AddrSpace::Handles,
                                     happ,
                                 )))
-                            },
-                        ),
+                            }),
                     );
                 }
                 &DataInstKind::QPtr(QPtrOp::BufferDynLen { fixed_base_size, dyn_unit_stride }) => {
@@ -1162,6 +1172,7 @@ impl<'a> GatherAccesses<'a> {
                     );
                 }
                 &DataInstKind::QPtr(QPtrOp::Offset(offset)) => {
+                    assert_eq!(per_output_accesses.len(), 1);
                     generate_accesses(
                         self,
                         data_inst_def.inputs[0],
@@ -1175,19 +1186,23 @@ impl<'a> GatherAccesses<'a> {
                             })
                             .and_then(|offset| {
                                 offset_accesses(
-                                    output_accesses
+                                    per_output_accesses[0]
+                                        .take()
                                         .unwrap_or(Ok(MemAccesses::Data(DataHapp::DEAD)))?,
                                     offset,
                                 )
                             }),
                     );
                 }
                 DataInstKind::QPtr(QPtrOp::DynOffset { stride, index_bounds }) => {
+                    assert_eq!(per_output_accesses.len(), 1);
                     generate_accesses(
                         self,
                         data_inst_def.inputs[0],
-                        output_accesses.unwrap_or(Ok(MemAccesses::Data(DataHapp::DEAD))).and_then(
-                            |accesses| {
+                        per_output_accesses[0]
+                            .take()
+                            .unwrap_or(Ok(MemAccesses::Data(DataHapp::DEAD)))
+                            .and_then(|accesses| {
                                 let happ = match accesses {
                                     MemAccesses::Handles(_) => {
                                         return Err(AnalysisError(Diag::bug([
@@ -1241,8 +1256,7 @@ impl<'a> GatherAccesses<'a> {
                                         stride: *stride,
                                     },
                                 }))
-                            },
-                        ),
+                            }),
                     );
                 }
                 DataInstKind::Mem(op @ (MemOp::Load { offset } | MemOp::Store { offset })) => {
@@ -1358,7 +1372,7 @@ impl<'a> GatherAccesses<'a> {
                     }
                 }
 
-                DataInstKind::SpvInst(_) | DataInstKind::SpvExtInst { .. } => {
+                DataInstKind::SpvInst(..) | DataInstKind::SpvExtInst { .. } => {
                     for attr in &cx[data_inst_def.attrs].attrs {
                         if let Attr::QPtr(QPtrAttr::ToSpvPtrInput { input_idx, pointee }) = *attr {
                             let ty = pointee.0;