Combine TypeCtor/ConstCtor and "ctor args" into a single TypeKind/ConstKind.

CHANGELOG.md

@@ -34,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased] - ReleaseDate
 
 ### Changed 🛠
+- [PR#51](https://github.com/EmbarkStudios/spirt/pull/51) combined `TypeCtor`/`ConstCtor`
+  and their respective "ctor args", into a single unified `TypeKind`/`ConstKind`
 - [PR#48](https://github.com/EmbarkStudios/spirt/pull/48) changed CFG structurization
   from "maximal loops" to "minimal loops" (computed using Tarjan's SCC algorithm),
   and added `OpLoopMerge` support on top (by extending a "minimal loop" as needed)

src/cfg.rs

@@ -1,13 +1,15 @@
 //! Control-flow graph (CFG) abstractions and utilities.
 
 use crate::{
-    spv, AttrSet, Const, ConstCtor, ConstDef, Context, ControlNode, ControlNodeDef,
+    spv, AttrSet, Const, ConstDef, ConstKind, Context, ControlNode, ControlNodeDef,
     ControlNodeKind, ControlNodeOutputDecl, ControlRegion, ControlRegionDef, EntityList,
-    EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, SelectionKind, Type, TypeCtor,
-    TypeDef, Value,
+    EntityOrientedDenseMap, FuncDefBody, FxIndexMap, FxIndexSet, SelectionKind, Type, TypeKind,
+    Value,
 };
+use itertools::Either;
 use smallvec::SmallVec;
 use std::mem;
+use std::rc::Rc;
 
 /// The control-flow graph (CFG) of a function, as control-flow instructions
 /// ([`ControlInst`]s) attached to [`ControlRegion`]s, as an "action on exit", i.e.
@@ -128,7 +130,6 @@ mod sealed {
         }
     }
 }
-use itertools::Either;
 use sealed::IncomingEdgeCount;
 
 struct TraversalState<PreVisit: FnMut(ControlRegion), PostVisit: FnMut(ControlRegion)> {
@@ -544,22 +545,29 @@ impl<'a> Structurizer<'a> {
     pub fn new(cx: &'a Context, func_def_body: &'a mut FuncDefBody) -> Self {
         // FIXME(eddyb) SPIR-T should have native booleans itself.
         let wk = &spv::spec::Spec::get().well_known;
-        let type_bool = cx.intern(TypeDef {
-            attrs: AttrSet::default(),
-            ctor: TypeCtor::SpvInst(wk.OpTypeBool.into()),
-            ctor_args: [].into_iter().collect(),
+        let type_bool = cx.intern(TypeKind::SpvInst {
+            spv_inst: wk.OpTypeBool.into(),
+            type_and_const_inputs: [].into_iter().collect(),
         });
         let const_true = cx.intern(ConstDef {
             attrs: AttrSet::default(),
             ty: type_bool,
-            ctor: ConstCtor::SpvInst(wk.OpConstantTrue.into()),
-            ctor_args: [].into_iter().collect(),
+            kind: ConstKind::SpvInst {
+                spv_inst_and_const_inputs: Rc::new((
+                    wk.OpConstantTrue.into(),
+                    [].into_iter().collect(),
+                )),
+            },
         });
         let const_false = cx.intern(ConstDef {
             attrs: AttrSet::default(),
             ty: type_bool,
-            ctor: ConstCtor::SpvInst(wk.OpConstantFalse.into()),
-            ctor_args: [].into_iter().collect(),
+            kind: ConstKind::SpvInst {
+                spv_inst_and_const_inputs: Rc::new((
+                    wk.OpConstantFalse.into(),
+                    [].into_iter().collect(),
+                )),
+            },
         });
 
         let (loop_header_to_exit_targets, incoming_edge_counts_including_loop_exits) =
@@ -1435,8 +1443,9 @@ impl<'a> Structurizer<'a> {
         self.cx.intern(ConstDef {
             attrs: AttrSet::default(),
             ty,
-            ctor: ConstCtor::SpvInst(wk.OpUndef.into()),
-            ctor_args: [].into_iter().collect(),
+            kind: ConstKind::SpvInst {
+                spv_inst_and_const_inputs: Rc::new((wk.OpUndef.into(), [].into_iter().collect())),
+            },
         })
     }
 }

src/lib.rs

@@ -173,6 +173,7 @@ pub mod spv;
 use smallvec::SmallVec;
 use std::borrow::Cow;
 use std::collections::BTreeSet;
+use std::rc::Rc;
 
 // HACK(eddyb) work around the lack of `FxIndex{Map,Set}` type aliases elsewhere.
 #[doc(hidden)]
@@ -457,13 +458,11 @@ pub use context::Type;
 #[derive(PartialEq, Eq, Hash)]
 pub struct TypeDef {
     pub attrs: AttrSet,
-    pub ctor: TypeCtor,
-    pub ctor_args: SmallVec<[TypeCtorArg; 2]>,
+    pub kind: TypeKind,
 }
 
-/// [`Type`] "constructor": a [`TypeDef`] wiithout any [`TypeCtorArg`]s ([`Type`]s/[`Const`]s).
 #[derive(Clone, PartialEq, Eq, Hash)]
-pub enum TypeCtor {
+pub enum TypeKind {
     /// "Quasi-pointer", an untyped pointer-like abstract scalar that can represent
     /// both memory locations (in any address space) and other kinds of locations
     /// (e.g. SPIR-V `OpVariable`s in non-memory "storage classes").
@@ -480,15 +479,28 @@ pub enum TypeCtor {
     // separately in e.g. `ControlRegionInputDecl`, might be a better approach?
     QPtr,
 
-    SpvInst(spv::Inst),
+    SpvInst {
+        spv_inst: spv::Inst,
+        // FIXME(eddyb) find a better name.
+        type_and_const_inputs: SmallVec<[TypeOrConst; 2]>,
+    },
 
-    /// The type of a [`ConstCtor::SpvStringLiteralForExtInst`] constant, i.e.
+    /// The type of a [`ConstKind::SpvStringLiteralForExtInst`] constant, i.e.
     /// a SPIR-V `OpString` with no actual type in SPIR-V.
     SpvStringLiteralForExtInst,
 }
 
+// HACK(eddyb) this behaves like an implicit conversion for `cx.intern(...)`.
+impl context::InternInCx<Type> for TypeKind {
+    fn intern_in_cx(self, cx: &Context) -> Type {
+        cx.intern(TypeDef { attrs: Default::default(), kind: self })
+    }
+}
+
+// HACK(eddyb) this is like `Either<Type, Const>`, only used in `TypeKind::SpvInst`,
+// and only because SPIR-V type definitions can references both types and consts.
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
-pub enum TypeCtorArg {
+pub enum TypeOrConst {
     Type(Type),
     Const(Const),
 }
@@ -503,16 +515,18 @@ pub use context::Const;
 pub struct ConstDef {
     pub attrs: AttrSet,
     pub ty: Type,
-    pub ctor: ConstCtor,
-    pub ctor_args: SmallVec<[Const; 2]>,
+    pub kind: ConstKind,
 }
 
-/// [`Const`] "constructor": a [`ConstDef`] wiithout any nested [`Const`]s.
 #[derive(Clone, PartialEq, Eq, Hash)]
-pub enum ConstCtor {
+pub enum ConstKind {
     PtrToGlobalVar(GlobalVar),
 
-    SpvInst(spv::Inst),
+    // HACK(eddyb) this is a fallback case that should become increasingly rare
+    // (especially wrt recursive consts), `Rc` means it can't bloat `ConstDef`.
+    SpvInst {
+        spv_inst_and_const_inputs: Rc<(spv::Inst, SmallVec<[Const; 4]>)>,
+    },
 
     /// SPIR-V `OpString`, but only when used as an operand for an `OpExtInst`,
     /// which can't have literals itself - for non-string literals `OpConstant*`

src/passes/qptr.rs

@@ -1,4 +1,4 @@
-//! [`QPtr`](crate::TypeCtor::QPtr) transforms.
+//! [`QPtr`](crate::TypeKind::QPtr) transforms.
 
 use crate::visit::{InnerVisit, Visitor};
 use crate::{qptr, DataInstForm};