Skip to content

print: attempt to improve GlobalVar printing by using named arguments. #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

print: attempt to improve GlobalVar printing by using named arguments. #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02cacc0
spv/lift: elide empty targets of an `if`-`else`/`switch`, as allowed …
eddyb Sep 9, 2025
dcd3b5b
print: attempt to improve `GlobalVar` printing by using named arguments.
eddyb Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ fn main() -> @location(0) i32 {
```cxx
#[spv.Decoration.Flat]
#[spv.Decoration.Location(Location: 0)]
global_var GV0 in spv.StorageClass.Output: s32
global_var GV0(spv.StorageClass.Output): s32

func F0() -> spv.OpTypeVoid {
loop(v0: s32 <- 1s32, v1: s32 <- 1s32) {
Expand Down
259 changes: 138 additions & 121 deletions src/print/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3380,139 +3380,148 @@ impl Print for GlobalVarDecl {

let wk = &spv::spec::Spec::get().well_known;

// HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but
// ideally the `GlobalVarDecl` would hold that type itself.
let type_ascription_suffix = match &printer.cx[*type_of_ptr_to].kind {
TypeKind::QPtr if shape.is_some() => match shape.unwrap() {
qptr::shapes::GlobalVarShape::Handles { handle, fixed_count } => {
let handle = match handle {
qptr::shapes::Handle::Opaque(ty) => ty.print(printer),
qptr::shapes::Handle::Buffer(addr_space, buf) => pretty::Fragment::new([
printer.declarative_keyword_style().apply("buffer").into(),
pretty::join_comma_sep(
"(",
[
addr_space.print(printer),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("size"),
pretty::Fragment::new(
Some(buf.fixed_base.size)
.filter(|&base_size| {
base_size > 0 || buf.dyn_unit_stride.is_none()
})
.map(|base_size| {
printer
.numeric_literal_style()
.apply(base_size.to_string())
.into()
})
.into_iter()
.chain(buf.dyn_unit_stride.map(|stride| {
pretty::Fragment::new([
"N × ".into(),
printer
.numeric_literal_style()
.apply(stride.to_string()),
])
}))
.intersperse_with(|| " + ".into()),
),
]),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("align"),
printer
.numeric_literal_style()
.apply(buf.fixed_base.align.to_string())
.into(),
]),
],
")",
),
]),
};
// HACK(eddyb) to avoid too many syntax variations, most details (other
// than the type, if present) use named arguments in `GV123(...)`.
let mut details = SmallVec::<[_; 4]>::new();

let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
handle
} else {
pretty::Fragment::new([
"[".into(),
fixed_count
.map(|count| {
pretty::Fragment::new([
printer.numeric_literal_style().apply(count.to_string()),
" × ".into(),
])
})
.unwrap_or_default(),
handle,
"]".into(),
])
};
pretty::join_space(":", [handles])
}
qptr::shapes::GlobalVarShape::UntypedData(mem_layout) => pretty::Fragment::new([
" ".into(),
printer.declarative_keyword_style().apply("layout").into(),
pretty::join_comma_sep(
"(",
[
pretty::Fragment::new([
printer.pretty_named_argument_prefix("size"),
printer
.numeric_literal_style()
.apply(mem_layout.size.to_string())
.into(),
]),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("align"),
printer
.numeric_literal_style()
.apply(mem_layout.align.to_string())
.into(),
]),
],
")",
),
]),
qptr::shapes::GlobalVarShape::TypedInterface(ty) => {
printer.pretty_type_ascription_suffix(ty)
}
},
TypeKind::SpvInst { spv_inst, type_and_const_inputs }
match addr_space {
AddrSpace::Handles => {}
AddrSpace::SpvStorageClass(_) => {
details.push(addr_space.print(printer));
}
}

// FIXME(eddyb) should this be a helper on `Printer`?
let num_lit = |x: u32| printer.numeric_literal_style().apply(format!("{x}")).into();

// FIXME(eddyb) should the pointer type be shown as something like
// `&GV123: OpTypePointer(..., T123)` *after* the variable definition?
// (but each reference can technically have a different type...)
let (qptr_shape, spv_ptr_pointee_type) = match &printer.cx[*type_of_ptr_to].kind {
TypeKind::QPtr => (shape.as_ref(), None),

// HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but
// ideally the `GlobalVarDecl` would hold that type itself.
TypeKind::SpvInst { spv_inst, type_and_const_inputs, .. }
if spv_inst.opcode == wk.OpTypePointer =>
{
match type_and_const_inputs[..] {
[TypeOrConst::Type(ty)] => printer.pretty_type_ascription_suffix(ty),
_ => unreachable!(),
[TypeOrConst::Type(pointee_type)] => (None, Some(pointee_type)),
_ => (None, None),
}
}
_ => pretty::Fragment::new([
": ".into(),
printer.error_style().apply("pointee_type_of").into(),
"(".into(),
type_of_ptr_to.print(printer),
")".into(),
]),

_ => (None, None),
};
let addr_space_suffix = match addr_space {
AddrSpace::Handles => pretty::Fragment::default(),
AddrSpace::SpvStorageClass(_) => {
pretty::Fragment::new([" in ".into(), addr_space.print(printer)])
let ascribe_type = match qptr_shape {
Some(qptr::shapes::GlobalVarShape::Handles { handle, fixed_count }) => {
let handle = match handle {
qptr::shapes::Handle::Opaque(ty) => ty.print(printer),
qptr::shapes::Handle::Buffer(addr_space, buf) => pretty::Fragment::new([
printer.declarative_keyword_style().apply("buffer").into(),
pretty::join_comma_sep(
"(",
[
addr_space.print(printer),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("size"),
pretty::Fragment::new(
[
Some(buf.fixed_base.size)
.filter(|&base_size| {
base_size > 0 || buf.dyn_unit_stride.is_none()
})
.map(num_lit),
buf.dyn_unit_stride.map(|stride| {
pretty::Fragment::new([
"N × ".into(),
num_lit(stride.get()),
])
}),
]
.into_iter()
.flatten()
.intersperse_with(|| " + ".into()),
),
]),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("align"),
num_lit(buf.fixed_base.align),
]),
],
")",
),
]),
};

let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
handle
} else {
pretty::Fragment::new([
"[".into(),
fixed_count
.map(|count| {
pretty::Fragment::new([num_lit(count.get()), " × ".into()])
})
.unwrap_or_default(),
handle,
"]".into(),
])
};
Some(handles)
}
Some(qptr::shapes::GlobalVarShape::UntypedData(mem_layout)) => {
details.extend([
pretty::Fragment::new([
printer.pretty_named_argument_prefix("size"),
num_lit(mem_layout.size),
]),
pretty::Fragment::new([
printer.pretty_named_argument_prefix("align"),
num_lit(mem_layout.align),
]),
]);
None
}
Some(qptr::shapes::GlobalVarShape::TypedInterface(ty)) => Some(ty.print(printer)),

None => Some(match spv_ptr_pointee_type {
Some(ty) => ty.print(printer),
None => pretty::Fragment::new([
printer.error_style().apply("pointee_type_of").into(),
"(".into(),
type_of_ptr_to.print(printer),
")".into(),
]),
}),
};
let header = pretty::Fragment::new([addr_space_suffix, type_ascription_suffix]);

let maybe_rhs = match def {
let import = match def {
// FIXME(eddyb) deduplicate with `FuncDecl`, and maybe consider
// putting the import *before* the declaration, to end up with:
// import "..."
// as global_var GV...
DeclDef::Imported(import) => Some(import.print(printer)),
DeclDef::Present(GlobalVarDefBody { initializer }) => {
// FIXME(eddyb) `global_varX in AS: T = Y` feels a bit wonky for
// the initializer, but it's cleaner than obvious alternatives.
initializer.map(|initializer| initializer.print(printer))
if let Some(initializer) = initializer {
details.push(pretty::Fragment::new([
printer.pretty_named_argument_prefix("init"),
initializer.print(printer),
]));
}
None
}
};
let body = maybe_rhs.map(|rhs| pretty::Fragment::new(["= ".into(), rhs]));

let def_without_name = pretty::Fragment::new([header, pretty::join_space("", body)]);
let def_without_name = pretty::Fragment::new(
[
(!details.is_empty()).then(|| pretty::join_comma_sep("(", details, ")")),
ascribe_type.map(|ty| pretty::join_space(":", [ty])),
import.map(|import| pretty::Fragment::new([" = ".into(), import])),
]
.into_iter()
.flatten(),
);

AttrsAndDef { attrs: attrs.print(printer), def_without_name }
}
Expand Down Expand Up @@ -3557,9 +3566,17 @@ impl Print for FuncDecl {
]);

let def_without_name = match def {
DeclDef::Imported(import) => {
pretty::Fragment::new([sig, " = ".into(), import.print(printer)])
}
// FIXME(eddyb) deduplicate with `GlobalVarDecl`, and maybe consider
// putting the import *before* the declaration, to end up with:
// import "..."
// as func F...
DeclDef::Imported(import) => pretty::Fragment::new([
sig,
pretty::join_space(
"",
[pretty::Fragment::new(["= ".into(), import.print(printer)])],
),
]),

// FIXME(eddyb) this can probably go into `impl Print for FuncDefBody`.
DeclDef::Present(def) => pretty::Fragment::new([
Expand Down
62 changes: 56 additions & 6 deletions src/spv/lift.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,22 +863,72 @@ impl<'a> FuncLifting<'a> {
// HACK(eddyb) this takes advantage of `blocks` being an `IndexMap`,
// to iterate at the same time as mutating other entries.
for block_idx in (0..blocks.len()).rev() {
let BlockLifting { terminator: original_terminator, .. } = &blocks[block_idx];
// HACK(eddyb) elide empty cases of an `if`-`else`/`switch`, as
// SPIR-V allows their targets to just be the whole merge block
// (the same one that `OpSelectionMerge` describes).
let block = &blocks[block_idx];
if let (cfg::ControlInstKind::SelectBranch(_), Some(Merge::Selection(merge_point))) =
(&*block.terminator.kind, block.terminator.merge)
{
for target_idx in 0..block.terminator.targets.len() {
let block = &blocks[block_idx];
let target = block.terminator.targets[target_idx];
if !block
.terminator
.target_phi_values
.get(&target)
.copied()
.unwrap_or_default()
.is_empty()
{
continue;
}

let target_is_trivial_branch = {
let BlockLifting {
phis,
insts,
terminator:
Terminator { attrs, kind, inputs, targets, target_phi_values, merge },
} = &blocks[&target];

(phis.is_empty()
&& insts.iter().all(|insts| insts.is_empty())
&& *attrs == AttrSet::default()
&& matches!(**kind, cfg::ControlInstKind::Branch)
&& inputs.is_empty()
&& targets.len() == 1
&& target_phi_values.is_empty()
&& merge.is_none())
.then(|| targets[0])
};
if let Some(target_of_target) = target_is_trivial_branch {
// FIXME(eddyb) what does it mean for this to not be true?
// (can it even happen?)
if target_of_target == merge_point {
blocks[block_idx].terminator.targets[target_idx] = target_of_target;
*use_counts.get_mut(&target).unwrap() -= 1;
*use_counts.get_mut(&target_of_target).unwrap() += 1;
}
}
}
}

let block = &blocks[block_idx];
let is_trivial_branch = {
let Terminator { attrs, kind, inputs, targets, target_phi_values, merge } =
original_terminator;
&block.terminator;

*attrs == AttrSet::default()
(*attrs == AttrSet::default()
&& matches!(**kind, cfg::ControlInstKind::Branch)
&& inputs.is_empty()
&& targets.len() == 1
&& target_phi_values.is_empty()
&& merge.is_none()
&& merge.is_none())
.then(|| targets[0])
};

if is_trivial_branch {
let target = original_terminator.targets[0];
if let Some(target) = is_trivial_branch {
let target_use_count = use_counts.get_mut(&target).unwrap();

if *target_use_count == 1 {
Expand Down
Loading