Skip to content

Commit 84ecf4c

Browse files
eddybeddyb
committed
print: attempt to improve GlobalVar printing by using named arguments.
1 parent b552a64 commit 84ecf4c

File tree

2 files changed

+130
-118
lines changed

2 files changed

+130
-118
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ fn main() -> @location(0) i32 {
133133
```cxx
134134
#[spv.Decoration.Flat]
135135
#[spv.Decoration.Location(Location: 0)]
136-
global_var GV0 in spv.StorageClass.Output: s32
136+
global_var GV0(spv.StorageClass.Output): s32
137137

138138
func F0() -> spv.OpTypeVoid {
139139
loop(v0: s32 <- 1s32, v1: s32 <- 1s32) {

‎src/print/mod.rs‎

Lines changed: 129 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -3386,144 +3386,148 @@ impl Print for GlobalVarDecl {
33863386

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

3389-
// HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but
3390-
// ideally the `GlobalVarDecl` would hold that type itself.
3391-
let type_ascription_suffix = match &printer.cx[*type_of_ptr_to].kind {
3392-
TypeKind::QPtr if shape.is_some() => match shape.unwrap() {
3393-
crate::mem::shapes::GlobalVarShape::Handles { handle, fixed_count } => {
3394-
let handle = match handle {
3395-
crate::mem::shapes::Handle::Opaque(ty) => ty.print(printer),
3396-
crate::mem::shapes::Handle::Buffer(addr_space, buf) => {
3397-
pretty::Fragment::new([
3398-
printer.declarative_keyword_style().apply("buffer").into(),
3399-
pretty::join_comma_sep(
3400-
"(",
3401-
[
3402-
addr_space.print(printer),
3403-
pretty::Fragment::new([
3404-
printer.pretty_named_argument_prefix("size"),
3405-
pretty::Fragment::new(
3406-
Some(buf.fixed_base.size)
3407-
.filter(|&base_size| {
3408-
base_size > 0
3409-
|| buf.dyn_unit_stride.is_none()
3410-
})
3411-
.map(|base_size| {
3412-
printer
3413-
.numeric_literal_style()
3414-
.apply(base_size.to_string())
3415-
.into()
3416-
})
3417-
.into_iter()
3418-
.chain(buf.dyn_unit_stride.map(|stride| {
3419-
pretty::Fragment::new([
3420-
"N × ".into(),
3421-
printer
3422-
.numeric_literal_style()
3423-
.apply(stride.to_string()),
3424-
])
3425-
}))
3426-
.intersperse_with(|| " + ".into()),
3427-
),
3428-
]),
3429-
pretty::Fragment::new([
3430-
printer.pretty_named_argument_prefix("align"),
3431-
printer
3432-
.numeric_literal_style()
3433-
.apply(buf.fixed_base.align.to_string())
3434-
.into(),
3435-
]),
3436-
],
3437-
")",
3438-
),
3439-
])
3440-
}
3441-
};
3389+
// HACK(eddyb) to avoid too many syntax variations, most details (other
3390+
// than the type, if present) use named arguments in `GV123(...)`.
3391+
let mut details = SmallVec::<[_; 4]>::new();
34423392

3443-
let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
3444-
handle
3445-
} else {
3446-
pretty::Fragment::new([
3447-
"[".into(),
3448-
fixed_count
3449-
.map(|count| {
3450-
pretty::Fragment::new([
3451-
printer.numeric_literal_style().apply(count.to_string()),
3452-
" × ".into(),
3453-
])
3454-
})
3455-
.unwrap_or_default(),
3456-
handle,
3457-
"]".into(),
3458-
])
3459-
};
3460-
pretty::join_space(":", [handles])
3393+
match addr_space {
3394+
AddrSpace::Handles => {}
3395+
AddrSpace::SpvStorageClass(_) => {
3396+
details.push(addr_space.print(printer));
3397+
}
3398+
}
3399+
3400+
// FIXME(eddyb) should this be a helper on `Printer`?
3401+
let num_lit = |x: u32| printer.numeric_literal_style().apply(format!("{x}")).into();
3402+
3403+
// FIXME(eddyb) should the pointer type be shown as something like
3404+
// `&GV123: OpTypePointer(..., T123)` *after* the variable definition?
3405+
// (but each reference can technically have a different type...)
3406+
let (qptr_shape, spv_ptr_pointee_type) = match &printer.cx[*type_of_ptr_to].kind {
3407+
TypeKind::QPtr => (shape.as_ref(), None),
3408+
3409+
// HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but
3410+
// ideally the `GlobalVarDecl` would hold that type itself.
3411+
TypeKind::SpvInst { spv_inst, type_and_const_inputs, .. }
3412+
if spv_inst.opcode == wk.OpTypePointer =>
3413+
{
3414+
match type_and_const_inputs[..] {
3415+
[TypeOrConst::Type(pointee_type)] => (None, Some(pointee_type)),
3416+
_ => (None, None),
34613417
}
3462-
crate::mem::shapes::GlobalVarShape::UntypedData(mem_layout) => {
3463-
pretty::Fragment::new([
3464-
" ".into(),
3465-
printer.declarative_keyword_style().apply("layout").into(),
3418+
}
3419+
3420+
_ => (None, None),
3421+
};
3422+
let ascribe_type = match qptr_shape {
3423+
Some(crate::mem::shapes::GlobalVarShape::Handles { handle, fixed_count }) => {
3424+
let handle = match handle {
3425+
crate::mem::shapes::Handle::Opaque(ty) => ty.print(printer),
3426+
crate::mem::shapes::Handle::Buffer(addr_space, buf) => pretty::Fragment::new([
3427+
printer.declarative_keyword_style().apply("buffer").into(),
34663428
pretty::join_comma_sep(
34673429
"(",
34683430
[
3431+
addr_space.print(printer),
34693432
pretty::Fragment::new([
34703433
printer.pretty_named_argument_prefix("size"),
3471-
printer
3472-
.numeric_literal_style()
3473-
.apply(mem_layout.size.to_string())
3474-
.into(),
3434+
pretty::Fragment::new(
3435+
[
3436+
Some(buf.fixed_base.size)
3437+
.filter(|&base_size| {
3438+
base_size > 0 || buf.dyn_unit_stride.is_none()
3439+
})
3440+
.map(num_lit),
3441+
buf.dyn_unit_stride.map(|stride| {
3442+
pretty::Fragment::new([
3443+
"N × ".into(),
3444+
num_lit(stride.get()),
3445+
])
3446+
}),
3447+
]
3448+
.into_iter()
3449+
.flatten()
3450+
.intersperse_with(|| " + ".into()),
3451+
),
34753452
]),
34763453
pretty::Fragment::new([
34773454
printer.pretty_named_argument_prefix("align"),
3478-
printer
3479-
.numeric_literal_style()
3480-
.apply(mem_layout.align.to_string())
3481-
.into(),
3455+
num_lit(buf.fixed_base.align),
34823456
]),
34833457
],
34843458
")",
34853459
),
3460+
]),
3461+
};
3462+
3463+
let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
3464+
handle
3465+
} else {
3466+
pretty::Fragment::new([
3467+
"[".into(),
3468+
fixed_count
3469+
.map(|count| {
3470+
pretty::Fragment::new([num_lit(count.get()), " × ".into()])
3471+
})
3472+
.unwrap_or_default(),
3473+
handle,
3474+
"]".into(),
34863475
])
3487-
}
3488-
crate::mem::shapes::GlobalVarShape::TypedInterface(ty) => {
3489-
printer.pretty_type_ascription_suffix(ty)
3490-
}
3491-
},
3492-
TypeKind::SpvInst { spv_inst, type_and_const_inputs }
3493-
if spv_inst.opcode == wk.OpTypePointer =>
3494-
{
3495-
match type_and_const_inputs[..] {
3496-
[TypeOrConst::Type(ty)] => printer.pretty_type_ascription_suffix(ty),
3497-
_ => unreachable!(),
3498-
}
3476+
};
3477+
Some(handles)
34993478
}
3500-
_ => pretty::Fragment::new([
3501-
": ".into(),
3502-
printer.error_style().apply("pointee_type_of").into(),
3503-
"(".into(),
3504-
type_of_ptr_to.print(printer),
3505-
")".into(),
3506-
]),
3507-
};
3508-
let addr_space_suffix = match addr_space {
3509-
AddrSpace::Handles => pretty::Fragment::default(),
3510-
AddrSpace::SpvStorageClass(_) => {
3511-
pretty::Fragment::new([" in ".into(), addr_space.print(printer)])
3479+
Some(crate::mem::shapes::GlobalVarShape::UntypedData(mem_layout)) => {
3480+
details.extend([
3481+
pretty::Fragment::new([
3482+
printer.pretty_named_argument_prefix("size"),
3483+
num_lit(mem_layout.size),
3484+
]),
3485+
pretty::Fragment::new([
3486+
printer.pretty_named_argument_prefix("align"),
3487+
num_lit(mem_layout.align),
3488+
]),
3489+
]);
3490+
None
35123491
}
3492+
Some(crate::mem::shapes::GlobalVarShape::TypedInterface(ty)) => Some(ty.print(printer)),
3493+
3494+
None => Some(match spv_ptr_pointee_type {
3495+
Some(ty) => ty.print(printer),
3496+
None => pretty::Fragment::new([
3497+
printer.error_style().apply("pointee_type_of").into(),
3498+
"(".into(),
3499+
type_of_ptr_to.print(printer),
3500+
")".into(),
3501+
]),
3502+
}),
35133503
};
3514-
let header = pretty::Fragment::new([addr_space_suffix, type_ascription_suffix]);
35153504

3516-
let maybe_rhs = match def {
3505+
let import = match def {
3506+
// FIXME(eddyb) deduplicate with `FuncDecl`, and maybe consider
3507+
// putting the import *before* the declaration, to end up with:
3508+
// import "..."
3509+
// as global_var GV...
35173510
DeclDef::Imported(import) => Some(import.print(printer)),
35183511
DeclDef::Present(GlobalVarDefBody { initializer }) => {
3519-
// FIXME(eddyb) `global_varX in AS: T = Y` feels a bit wonky for
3520-
// the initializer, but it's cleaner than obvious alternatives.
3521-
initializer.map(|initializer| initializer.print(printer))
3512+
if let Some(initializer) = initializer {
3513+
details.push(pretty::Fragment::new([
3514+
printer.pretty_named_argument_prefix("init"),
3515+
initializer.print(printer),
3516+
]));
3517+
}
3518+
None
35223519
}
35233520
};
3524-
let body = maybe_rhs.map(|rhs| pretty::Fragment::new(["= ".into(), rhs]));
35253521

3526-
let def_without_name = pretty::Fragment::new([header, pretty::join_space("", body)]);
3522+
let def_without_name = pretty::Fragment::new(
3523+
[
3524+
(!details.is_empty()).then(|| pretty::join_comma_sep("(", details, ")")),
3525+
ascribe_type.map(|ty| pretty::join_space(":", [ty])),
3526+
import.map(|import| pretty::Fragment::new([" = ".into(), import])),
3527+
]
3528+
.into_iter()
3529+
.flatten(),
3530+
);
35273531

35283532
AttrsAndDef { attrs: attrs.print(printer), def_without_name }
35293533
}
@@ -3568,9 +3572,17 @@ impl Print for FuncDecl {
35683572
]);
35693573

35703574
let def_without_name = match def {
3571-
DeclDef::Imported(import) => {
3572-
pretty::Fragment::new([sig, " = ".into(), import.print(printer)])
3573-
}
3575+
// FIXME(eddyb) deduplicate with `GlobalVarDecl`, and maybe consider
3576+
// putting the import *before* the declaration, to end up with:
3577+
// import "..."
3578+
// as func F...
3579+
DeclDef::Imported(import) => pretty::Fragment::new([
3580+
sig,
3581+
pretty::join_space(
3582+
"",
3583+
[pretty::Fragment::new(["= ".into(), import.print(printer)])],
3584+
),
3585+
]),
35743586

35753587
// FIXME(eddyb) this can probably go into `impl Print for FuncDefBody`.
35763588
DeclDef::Present(def) => pretty::Fragment::new([

0 commit comments

Comments
 (0)