diff --git a/README.md b/README.md index 2e93bc09..9db56ee4 100644 --- a/README.md +++ b/README.md @@ -136,14 +136,14 @@ fn main() -> @location(0) i32 { global_var GV0 in spv.StorageClass.Output: s32 func F0() -> spv.OpTypeVoid { - (_: s32, _: s32, v0: s32) = loop(v1: s32 <- 1s32, v2: s32 <- 1s32, _: s32 <- spv.OpUndef: s32) { + (_: s32, _: s32, v0: s32) = loop(v1: s32 <- 1s32, v2: s32 <- 1s32, _: s32 <- undef: s32) { v3 = spv.OpSLessThan(v2, 10s32): bool (v4: s32, v5: s32) = if v3 { v6 = spv.OpIMul(v1, v2): s32 v7 = spv.OpIAdd(v2, 1s32): s32 (v6, v7) } else { - (spv.OpUndef: s32, spv.OpUndef: s32) + (undef: s32, undef: s32) } (v4, v5, v1) -> (v1, v2, _) } while v3 diff --git a/src/cf/structurize.rs b/src/cf/structurize.rs index 46a0d09d..f8e08c0d 100644 --- a/src/cf/structurize.rs +++ b/src/cf/structurize.rs @@ -1939,14 +1939,6 @@ impl<'a> Structurizer<'a> { /// Create an undefined constant (as a placeholder where a value needs to be /// present, but won't actually be used), of type `ty`. fn const_undef(&self, ty: Type) -> Const { - // FIXME(eddyb) SPIR-T should have native undef itself. - let wk = &spv::spec::Spec::get().well_known; - self.cx.intern(ConstDef { - attrs: AttrSet::default(), - ty, - kind: ConstKind::SpvInst { - spv_inst_and_const_inputs: Rc::new((wk.OpUndef.into(), [].into_iter().collect())), - }, - }) + self.cx.intern(ConstDef { attrs: AttrSet::default(), ty, kind: ConstKind::Undef }) } } diff --git a/src/lib.rs b/src/lib.rs index 7c4a1771..52c6ce1c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -593,6 +593,12 @@ pub struct ConstDef { #[derive(Clone, PartialEq, Eq, Hash)] pub enum ConstKind { + /// Undeterminate value (i.e. SPIR-V `OpUndef`, LLVM `undef`). + // + // FIXME(eddyb) could it be possible to adopt LLVM's newer `poison`+`freeze` + // model, without being forced to never lift back to `OpUndef`? + Undef, + // FIXME(eddyb) maybe merge these? however, their connection is somewhat // tenuous (being one of the LLVM-isms SPIR-V inherited, among other things), // there's still the need to rename "global variable" post-`Var`-refactor, diff --git a/src/print/mod.rs b/src/print/mod.rs index 3da90cb9..6124ceee 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -3292,12 +3292,17 @@ impl Print for ConstDef { AttrsAndDef { attrs: attrs.print(printer), def_without_name: compact_def.unwrap_or_else(|| match kind { + ConstKind::Undef => pretty::Fragment::new([ + printer.imperative_keyword_style().apply("undef").into(), + printer.pretty_type_ascription_suffix(*ty), + ]), &ConstKind::PtrToGlobalVar(gv) => { pretty::Fragment::new(["&".into(), gv.print(printer)]) } &ConstKind::PtrToFunc(func) => { pretty::Fragment::new(["&".into(), func.print(printer)]) } + ConstKind::SpvInst { spv_inst_and_const_inputs } => { let (spv_inst, const_inputs) = &**spv_inst_and_const_inputs; pretty::Fragment::new([ @@ -4087,6 +4092,10 @@ impl FuncAt<'_, DataInst> { let pseudo_imm_from_value = |v: Value| { if let Value::Const(ct) = v { match &printer.cx[ct].kind { + ConstKind::Undef + | ConstKind::PtrToGlobalVar(_) + | ConstKind::PtrToFunc(_) => {} + &ConstKind::SpvStringLiteralForExtInst(s) => { return Some(PseudoImm::Str(&printer.cx[s])); } @@ -4101,7 +4110,6 @@ impl FuncAt<'_, DataInst> { } } } - ConstKind::PtrToGlobalVar(_) | ConstKind::PtrToFunc(_) => {} } } None diff --git a/src/spv/canonical.rs b/src/spv/canonical.rs new file mode 100644 index 00000000..45a0424a --- /dev/null +++ b/src/spv/canonical.rs @@ -0,0 +1,71 @@ +//! Bidirectional (SPIR-V <-> SPIR-T) "canonical mappings". +//! +//! Both directions are defined close together as much as possible, to: +//! - limit code duplication, making it easy to add more mappings +//! - limit how much they could even go out of sync over time +//! - prevent naming e.g. SPIR-V opcodes, outside canonicalization +// +// FIXME(eddyb) should interning attempts check/apply these canonicalizations? + +use crate::ConstKind; +use crate::spv::{self, spec}; +use lazy_static::lazy_static; + +// FIXME(eddyb) these ones could maybe make use of build script generation. +macro_rules! def_mappable_ops { + ($($op:ident),+ $(,)?) => { + #[allow(non_snake_case)] + struct MappableOps { + $($op: spec::Opcode,)+ + } + impl MappableOps { + #[inline(always)] + #[must_use] + pub fn get() -> &'static MappableOps { + lazy_static! { + static ref MAPPABLE_OPS: MappableOps = { + let spv_spec = spec::Spec::get(); + MappableOps { + $($op: spv_spec.instructions.lookup(stringify!($op)).unwrap(),)+ + } + }; + } + &MAPPABLE_OPS + } + } + }; +} +def_mappable_ops! { + OpUndef, +} + +// FIXME(eddyb) decide on a visibility scope - `pub(super)` avoids some mistakes +// (using these methods outside of `spv::{lower,lift}`), but may be too restrictive. +impl spv::Inst { + pub(super) fn as_canonical_const(&self) -> Option { + let Self { opcode, imms } = self; + let (&opcode, imms) = (opcode, &imms[..]); + + let mo = MappableOps::get(); + + if opcode == mo.OpUndef { + assert_eq!(imms.len(), 0); + Some(ConstKind::Undef) + } else { + None + } + } + + pub(super) fn from_canonical_const(const_kind: &ConstKind) -> Option { + let mo = MappableOps::get(); + + match const_kind { + ConstKind::Undef => Some(mo.OpUndef.into()), + + ConstKind::PtrToGlobalVar(_) + | ConstKind::PtrToFunc(_) + | ConstKind::SpvInst { .. } + | ConstKind::SpvStringLiteralForExtInst(_) => None, + } + } +} diff --git a/src/spv/lift.rs b/src/spv/lift.rs index e928fdaf..cb9ac3d3 100644 --- a/src/spv/lift.rs +++ b/src/spv/lift.rs @@ -146,7 +146,10 @@ impl Visitor<'_> for NeedsIdsCollector<'_> { } let ct_def = &self.cx[ct]; match ct_def.kind { - ConstKind::PtrToGlobalVar(_) | ConstKind::PtrToFunc(_) | ConstKind::SpvInst { .. } => { + ConstKind::Undef + | ConstKind::PtrToGlobalVar(_) + | ConstKind::PtrToFunc(_) + | ConstKind::SpvInst { .. } => { self.visit_const_def(ct_def); self.globals.insert(global); } @@ -1148,9 +1151,10 @@ impl LazyInst<'_, '_> { }; (gv_decl.attrs, import) } - ConstKind::PtrToFunc(_) | ConstKind::SpvInst { .. } => { - (ct_def.attrs, None) - } + + ConstKind::Undef + | ConstKind::PtrToFunc(_) + | ConstKind::SpvInst { .. } => (ct_def.attrs, None), // Not inserted into `globals` while visiting. ConstKind::SpvStringLiteralForExtInst(_) => unreachable!(), @@ -1246,8 +1250,19 @@ impl LazyInst<'_, '_> { }, Global::Const(ct) => { let ct_def = &cx[ct]; - match &ct_def.kind { - &ConstKind::PtrToGlobalVar(gv) => { + match spv::Inst::from_canonical_const(&ct_def.kind).ok_or(&ct_def.kind) { + Ok(spv_inst) => spv::InstWithIds { + without_ids: spv_inst, + result_type_id: Some(ids.globals[&Global::Type(ct_def.ty)]), + result_id, + ids: [].into_iter().collect(), + }, + + Err(ConstKind::Undef) => { + unreachable!("should've been handled as canonical") + } + + Err(&ConstKind::PtrToGlobalVar(gv)) => { assert!(ct_def.attrs == AttrSet::default()); let gv_decl = &module.global_vars[gv]; @@ -1280,14 +1295,14 @@ impl LazyInst<'_, '_> { } } - &ConstKind::PtrToFunc(func) => spv::InstWithIds { + Err(&ConstKind::PtrToFunc(func)) => spv::InstWithIds { without_ids: wk.OpConstantFunctionPointerINTEL.into(), result_type_id: Some(ids.globals[&Global::Type(ct_def.ty)]), result_id, ids: [ids.funcs[&func].func_id].into_iter().collect(), }, - ConstKind::SpvInst { spv_inst_and_const_inputs } => { + Err(ConstKind::SpvInst { spv_inst_and_const_inputs }) => { let (spv_inst, const_inputs) = &**spv_inst_and_const_inputs; spv::InstWithIds { without_ids: spv_inst.clone(), @@ -1301,7 +1316,7 @@ impl LazyInst<'_, '_> { } // Not inserted into `globals` while visiting. - ConstKind::SpvStringLiteralForExtInst(_) => unreachable!(), + Err(ConstKind::SpvStringLiteralForExtInst(_)) => unreachable!(), } } }, diff --git a/src/spv/lower.rs b/src/spv/lower.rs index 176d4aa8..9384ac7f 100644 --- a/src/spv/lower.rs +++ b/src/spv/lower.rs @@ -652,7 +652,29 @@ impl Module { id_defs.insert(id, IdDef::Const(ct)); Seq::TypeConstOrGlobalVar - } else if inst_category == spec::InstructionCategory::Const || opcode == wk.OpUndef { + } else if let Some(const_kind) = inst.as_canonical_const() { + let id = inst.result_id.unwrap(); + assert_eq!(inst.ids.len(), 0); + + // FIXME(eddyb) this is used below for sequencing, so maybe it + // may be useful to still have some access here to `wk.OpUndef`. + let is_op_undef = matches!(const_kind, ConstKind::Undef); + + let ct = cx.intern(ConstDef { + attrs: mem::take(&mut attrs), + ty: result_type.unwrap(), + kind: const_kind, + }); + id_defs.insert(id, IdDef::Const(ct)); + + if is_op_undef { + // `OpUndef` can appear either among constants, or in a + // function, so at most advance `seq` to globals. + seq.max(Some(Seq::TypeConstOrGlobalVar)).unwrap() + } else { + Seq::TypeConstOrGlobalVar + } + } else if inst_category == spec::InstructionCategory::Const { let id = inst.result_id.unwrap(); let const_inputs = inst .ids @@ -678,13 +700,7 @@ impl Module { }); id_defs.insert(id, IdDef::Const(ct)); - if opcode == wk.OpUndef { - // `OpUndef` can appear either among constants, or in a - // function, so at most advance `seq` to globals. - seq.max(Some(Seq::TypeConstOrGlobalVar)).unwrap() - } else { - Seq::TypeConstOrGlobalVar - } + Seq::TypeConstOrGlobalVar } else if opcode == wk.OpVariable && current_func_body.is_none() { let global_var_id = inst.result_id.unwrap(); let type_of_ptr_to_global_var = result_type.unwrap(); diff --git a/src/spv/mod.rs b/src/spv/mod.rs index bd9e7f50..3a945287 100644 --- a/src/spv/mod.rs +++ b/src/spv/mod.rs @@ -2,6 +2,7 @@ // NOTE(eddyb) all the modules are declared here, but they're documented "inside" // (i.e. using inner doc comments). +pub mod canonical; pub mod lift; pub mod lower; pub mod print; diff --git a/src/spv/spec.rs b/src/spv/spec.rs index 2161e28e..6daff6c1 100644 --- a/src/spv/spec.rs +++ b/src/spv/spec.rs @@ -136,7 +136,6 @@ def_well_known! { OpConstantFalse, OpConstantTrue, OpConstant, - OpUndef, OpConstantFunctionPointerINTEL, OpVariable, diff --git a/src/transform.rs b/src/transform.rs index 4cb38c04..0c76ace4 100644 --- a/src/transform.rs +++ b/src/transform.rs @@ -471,6 +471,9 @@ impl InnerTransform for ConstDef { attrs -> transformer.transform_attr_set_use(*attrs), ty -> transformer.transform_type_use(*ty), kind -> match kind { + ConstKind::Undef + | ConstKind::SpvStringLiteralForExtInst(_) => Transformed::Unchanged, + ConstKind::PtrToGlobalVar(gv) => transform!({ gv -> transformer.transform_global_var_use(*gv), } => ConstKind::PtrToGlobalVar(gv)), @@ -488,7 +491,6 @@ impl InnerTransform for ConstDef { spv_inst_and_const_inputs: Rc::new((spv_inst.clone(), new_iter.collect())), }) } - ConstKind::SpvStringLiteralForExtInst(_) => Transformed::Unchanged }, } => Self { attrs, diff --git a/src/visit.rs b/src/visit.rs index b48fcbf2..bd63094e 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -342,6 +342,8 @@ impl InnerVisit for ConstDef { visitor.visit_attr_set_use(*attrs); visitor.visit_type_use(*ty); match kind { + ConstKind::Undef | ConstKind::SpvStringLiteralForExtInst(_) => {} + &ConstKind::PtrToGlobalVar(gv) => visitor.visit_global_var_use(gv), &ConstKind::PtrToFunc(func) => visitor.visit_func_use(func), ConstKind::SpvInst { spv_inst_and_const_inputs } => { @@ -350,7 +352,6 @@ impl InnerVisit for ConstDef { visitor.visit_const_use(ct); } } - ConstKind::SpvStringLiteralForExtInst(_) => {} } } }