Skip to content

Commit f90c442

Browse files
eddybeddyb
committed
print: attempt to improve GlobalVar printing by using named arguments.
1 parent 5ce8e28 commit f90c442

File tree

2 files changed

+139
-122
lines changed

2 files changed

+139
-122
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: 138 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -3375,139 +3375,148 @@ impl Print for GlobalVarDecl {
33753375

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

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

3429-
let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
3430-
handle
3431-
} else {
3432-
pretty::Fragment::new([
3433-
"[".into(),
3434-
fixed_count
3435-
.map(|count| {
3436-
pretty::Fragment::new([
3437-
printer.numeric_literal_style().apply(count.to_string()),
3438-
" × ".into(),
3439-
])
3440-
})
3441-
.unwrap_or_default(),
3442-
handle,
3443-
"]".into(),
3444-
])
3445-
};
3446-
pretty::join_space(":", [handles])
3447-
}
3448-
qptr::shapes::GlobalVarShape::UntypedData(mem_layout) => pretty::Fragment::new([
3449-
" ".into(),
3450-
printer.declarative_keyword_style().apply("layout").into(),
3451-
pretty::join_comma_sep(
3452-
"(",
3453-
[
3454-
pretty::Fragment::new([
3455-
printer.pretty_named_argument_prefix("size"),
3456-
printer
3457-
.numeric_literal_style()
3458-
.apply(mem_layout.size.to_string())
3459-
.into(),
3460-
]),
3461-
pretty::Fragment::new([
3462-
printer.pretty_named_argument_prefix("align"),
3463-
printer
3464-
.numeric_literal_style()
3465-
.apply(mem_layout.align.to_string())
3466-
.into(),
3467-
]),
3468-
],
3469-
")",
3470-
),
3471-
]),
3472-
qptr::shapes::GlobalVarShape::TypedInterface(ty) => {
3473-
printer.pretty_type_ascription_suffix(ty)
3474-
}
3475-
},
3476-
TypeKind::SpvInst { spv_inst, type_and_const_inputs }
3382+
match addr_space {
3383+
AddrSpace::Handles => {}
3384+
AddrSpace::SpvStorageClass(_) => {
3385+
details.push(addr_space.print(printer));
3386+
}
3387+
}
3388+
3389+
// FIXME(eddyb) should this be a helper on `Printer`?
3390+
let num_lit = |x: u32| printer.numeric_literal_style().apply(format!("{x}")).into();
3391+
3392+
// FIXME(eddyb) should the pointer type be shown as something like
3393+
// `&GV123: OpTypePointer(..., T123)` *after* the variable definition?
3394+
// (but each reference can technically have a different type...)
3395+
let (qptr_shape, spv_ptr_pointee_type) = match &printer.cx[*type_of_ptr_to].kind {
3396+
TypeKind::QPtr => (shape.as_ref(), None),
3397+
3398+
// HACK(eddyb) get the pointee type from SPIR-V `OpTypePointer`, but
3399+
// ideally the `GlobalVarDecl` would hold that type itself.
3400+
TypeKind::SpvInst { spv_inst, type_and_const_inputs, .. }
34773401
if spv_inst.opcode == wk.OpTypePointer =>
34783402
{
34793403
match type_and_const_inputs[..] {
3480-
[TypeOrConst::Type(ty)] => printer.pretty_type_ascription_suffix(ty),
3481-
_ => unreachable!(),
3404+
[TypeOrConst::Type(pointee_type)] => (None, Some(pointee_type)),
3405+
_ => (None, None),
34823406
}
34833407
}
3484-
_ => pretty::Fragment::new([
3485-
": ".into(),
3486-
printer.error_style().apply("pointee_type_of").into(),
3487-
"(".into(),
3488-
type_of_ptr_to.print(printer),
3489-
")".into(),
3490-
]),
3408+
3409+
_ => (None, None),
34913410
};
3492-
let addr_space_suffix = match addr_space {
3493-
AddrSpace::Handles => pretty::Fragment::default(),
3494-
AddrSpace::SpvStorageClass(_) => {
3495-
pretty::Fragment::new([" in ".into(), addr_space.print(printer)])
3411+
let ascribe_type = match qptr_shape {
3412+
Some(qptr::shapes::GlobalVarShape::Handles { handle, fixed_count }) => {
3413+
let handle = match handle {
3414+
qptr::shapes::Handle::Opaque(ty) => ty.print(printer),
3415+
qptr::shapes::Handle::Buffer(addr_space, buf) => pretty::Fragment::new([
3416+
printer.declarative_keyword_style().apply("buffer").into(),
3417+
pretty::join_comma_sep(
3418+
"(",
3419+
[
3420+
addr_space.print(printer),
3421+
pretty::Fragment::new([
3422+
printer.pretty_named_argument_prefix("size"),
3423+
pretty::Fragment::new(
3424+
[
3425+
Some(buf.fixed_base.size)
3426+
.filter(|&base_size| {
3427+
base_size > 0 || buf.dyn_unit_stride.is_none()
3428+
})
3429+
.map(num_lit),
3430+
buf.dyn_unit_stride.map(|stride| {
3431+
pretty::Fragment::new([
3432+
"N × ".into(),
3433+
num_lit(stride.get()),
3434+
])
3435+
}),
3436+
]
3437+
.into_iter()
3438+
.flatten()
3439+
.intersperse_with(|| " + ".into()),
3440+
),
3441+
]),
3442+
pretty::Fragment::new([
3443+
printer.pretty_named_argument_prefix("align"),
3444+
num_lit(buf.fixed_base.align),
3445+
]),
3446+
],
3447+
")",
3448+
),
3449+
]),
3450+
};
3451+
3452+
let handles = if fixed_count.map_or(0, |c| c.get()) == 1 {
3453+
handle
3454+
} else {
3455+
pretty::Fragment::new([
3456+
"[".into(),
3457+
fixed_count
3458+
.map(|count| {
3459+
pretty::Fragment::new([num_lit(count.get()), " × ".into()])
3460+
})
3461+
.unwrap_or_default(),
3462+
handle,
3463+
"]".into(),
3464+
])
3465+
};
3466+
Some(handles)
3467+
}
3468+
Some(qptr::shapes::GlobalVarShape::UntypedData(mem_layout)) => {
3469+
details.extend([
3470+
pretty::Fragment::new([
3471+
printer.pretty_named_argument_prefix("size"),
3472+
num_lit(mem_layout.size),
3473+
]),
3474+
pretty::Fragment::new([
3475+
printer.pretty_named_argument_prefix("align"),
3476+
num_lit(mem_layout.align),
3477+
]),
3478+
]);
3479+
None
34963480
}
3481+
Some(qptr::shapes::GlobalVarShape::TypedInterface(ty)) => Some(ty.print(printer)),
3482+
3483+
None => Some(match spv_ptr_pointee_type {
3484+
Some(ty) => ty.print(printer),
3485+
None => pretty::Fragment::new([
3486+
printer.error_style().apply("pointee_type_of").into(),
3487+
"(".into(),
3488+
type_of_ptr_to.print(printer),
3489+
")".into(),
3490+
]),
3491+
}),
34973492
};
3498-
let header = pretty::Fragment::new([addr_space_suffix, type_ascription_suffix]);
34993493

3500-
let maybe_rhs = match def {
3494+
let import = match def {
3495+
// FIXME(eddyb) deduplicate with `FuncDecl`, and maybe consider
3496+
// putting the import *before* the declaration, to end up with:
3497+
// import "..."
3498+
// as global_var GV...
35013499
DeclDef::Imported(import) => Some(import.print(printer)),
35023500
DeclDef::Present(GlobalVarDefBody { initializer }) => {
3503-
// FIXME(eddyb) `global_varX in AS: T = Y` feels a bit wonky for
3504-
// the initializer, but it's cleaner than obvious alternatives.
3505-
initializer.map(|initializer| initializer.print(printer))
3501+
if let Some(initializer) = initializer {
3502+
details.push(pretty::Fragment::new([
3503+
printer.pretty_named_argument_prefix("init"),
3504+
initializer.print(printer),
3505+
]));
3506+
}
3507+
None
35063508
}
35073509
};
3508-
let body = maybe_rhs.map(|rhs| pretty::Fragment::new(["= ".into(), rhs]));
35093510

3510-
let def_without_name = pretty::Fragment::new([header, pretty::join_space("", body)]);
3511+
let def_without_name = pretty::Fragment::new(
3512+
[
3513+
(!details.is_empty()).then(|| pretty::join_comma_sep("(", details, ")")),
3514+
ascribe_type.map(|ty| pretty::join_space(":", [ty])),
3515+
import.map(|import| pretty::Fragment::new([" = ".into(), import])),
3516+
]
3517+
.into_iter()
3518+
.flatten(),
3519+
);
35113520

35123521
AttrsAndDef { attrs: attrs.print(printer), def_without_name }
35133522
}
@@ -3552,9 +3561,17 @@ impl Print for FuncDecl {
35523561
]);
35533562

35543563
let def_without_name = match def {
3555-
DeclDef::Imported(import) => {
3556-
pretty::Fragment::new([sig, " = ".into(), import.print(printer)])
3557-
}
3564+
// FIXME(eddyb) deduplicate with `GlobalVarDecl`, and maybe consider
3565+
// putting the import *before* the declaration, to end up with:
3566+
// import "..."
3567+
// as func F...
3568+
DeclDef::Imported(import) => pretty::Fragment::new([
3569+
sig,
3570+
pretty::join_space(
3571+
"",
3572+
[pretty::Fragment::new(["= ".into(), import.print(printer)])],
3573+
),
3574+
]),
35583575

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

0 commit comments

Comments
 (0)