Skip to content

Commit 4821521

Browse files
eddybeddyb
committed
print: attempt to improve GlobalVar printing by using named arguments.
1 parent e8757ad commit 4821521

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
@@ -3380,139 +3380,148 @@ impl Print for GlobalVarDecl {
33803380

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

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

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

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

3515-
let def_without_name = pretty::Fragment::new([header, pretty::join_space("", body)]);
3516+
let def_without_name = pretty::Fragment::new(
3517+
[
3518+
(!details.is_empty()).then(|| pretty::join_comma_sep("(", details, ")")),
3519+
ascribe_type.map(|ty| pretty::join_space(":", [ty])),
3520+
import.map(|import| pretty::Fragment::new([" = ".into(), import])),
3521+
]
3522+
.into_iter()
3523+
.flatten(),
3524+
);
35163525

35173526
AttrsAndDef { attrs: attrs.print(printer), def_without_name }
35183527
}
@@ -3557,9 +3566,17 @@ impl Print for FuncDecl {
35573566
]);
35583567

35593568
let def_without_name = match def {
3560-
DeclDef::Imported(import) => {
3561-
pretty::Fragment::new([sig, " = ".into(), import.print(printer)])
3562-
}
3569+
// FIXME(eddyb) deduplicate with `GlobalVarDecl`, and maybe consider
3570+
// putting the import *before* the declaration, to end up with:
3571+
// import "..."
3572+
// as func F...
3573+
DeclDef::Imported(import) => pretty::Fragment::new([
3574+
sig,
3575+
pretty::join_space(
3576+
"",
3577+
[pretty::Fragment::new(["= ".into(), import.print(printer)])],
3578+
),
3579+
]),
35633580

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

0 commit comments

Comments
 (0)