Skip to content

Commit 5c42614

Browse files
eddybeddyb
authored
"Unintern" DataInstForm (inlining its fields back into DataInstDef). (#12)
Effectively undoes: - EmbarkStudios/spirt#28 Some quick unscientific testing reveals no significant perf impact (i.e. the difference is lost in the noise). The motivation for undoing this interning is the prospect of combining `DataInstDef` into `NodeDef` (as mentioned in #7), and for unrelated pragmatic reasons, `NodeDef` can't have its outputs interned (though long-term maybe we could intern the `kind` field, if really necessary, assuming we first take "child regions" out of it). The one thing I realized too late there's no pre-existing consensus for, is the `output_type`, which used to be between `kind` and `inputs` (matching `DataInstFormDef`, in fact), while `NodeDef`'s `outputs` is the last field. The only argument to have outputs first is the `let (out0, out1) = foo(in0, in1);` style syntax (though pretty-printed SPIR-T omits the `let`), but I'm not sure that's worth flipping the order over.
2 parents 000ec78 + 43524d7 commit 5c42614

File tree

15 files changed

+140
-317
lines changed

15 files changed

+140
-317
lines changed

‎src/context.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,6 @@ interners! {
888888
crate::AttrSetDef,
889889
crate::TypeDef,
890890
crate::ConstDef,
891-
crate::DataInstFormDef,
892891
}
893892

894893
// FIXME(eddyb) consider a more uniform naming scheme than the combination
@@ -897,7 +896,6 @@ interners! {
897896
AttrSet default(crate::AttrSetDef::default()) => crate::AttrSetDef,
898897
Type => crate::TypeDef,
899898
Const => crate::ConstDef,
900-
DataInstForm => crate::DataInstFormDef,
901899
}
902900

903901
impl<I: sealed::Interned> InternInCx<I> for I::Def

‎src/func_at.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ impl FuncAt<'_, Value> {
116116
Value::NodeOutput { node, output_idx } => {
117117
self.at(node).def().outputs[output_idx as usize].ty
118118
}
119-
Value::DataInstOutput(inst) => cx[self.at(inst).def().form].output_type.unwrap(),
119+
Value::DataInstOutput(inst) => self.at(inst).def().output_type.unwrap(),
120120
}
121121
}
122122
}

‎src/lib.rs‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -934,26 +934,10 @@ pub use context::DataInst;
934934
pub struct DataInstDef {
935935
pub attrs: AttrSet,
936936

937-
pub form: DataInstForm,
937+
pub kind: DataInstKind,
938938

939939
// FIXME(eddyb) change the inline size of this to fit most instructions.
940940
pub inputs: SmallVec<[Value; 2]>,
941-
}
942-
943-
/// Interned handle for a [`DataInstFormDef`](crate::DataInstFormDef)
944-
/// (a "form", or "template", for [`DataInstDef`](crate::DataInstDef)s).
945-
pub use context::DataInstForm;
946-
947-
/// "Form" (or "template") definition for [`DataInstFormDef`]s, which includes
948-
/// most of their common *static* information (notably excluding `attrs`, as
949-
/// they vary more often due to handling diagnostics, debuginfo, refinement etc.).
950-
//
951-
// FIXME(eddyb) now that this is interned, try to find all the code that was
952-
// working around needing to borrow `DataInstKind`, just because it was owned
953-
// by a `FuncDefBody` (instead of interned in the `Context`).
954-
#[derive(Clone, PartialEq, Eq, Hash)]
955-
pub struct DataInstFormDef {
956-
pub kind: DataInstKind,
957941

958942
pub output_type: Option<Type>,
959943
}

‎src/passes/legalize.rs‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
use crate::visit::{InnerVisit, Visitor};
2-
use crate::{
3-
AttrSet, Const, Context, DataInstForm, DeclDef, Func, FxIndexSet, GlobalVar, Module, Type, cfg,
4-
};
2+
use crate::{AttrSet, Const, Context, DeclDef, Func, FxIndexSet, GlobalVar, Module, Type, cfg};
53

64
/// Apply the [`cfg::Structurizer`] algorithm to all function definitions in `module`.
75
pub fn structurize_func_cfgs(module: &mut Module) {
@@ -14,7 +12,6 @@ pub fn structurize_func_cfgs(module: &mut Module) {
1412

1513
seen_types: FxIndexSet::default(),
1614
seen_consts: FxIndexSet::default(),
17-
seen_data_inst_forms: FxIndexSet::default(),
1815
seen_global_vars: FxIndexSet::default(),
1916
seen_funcs: FxIndexSet::default(),
2017
};
@@ -37,7 +34,6 @@ struct ReachableUseCollector<'a> {
3734
// FIXME(eddyb) build some automation to avoid ever repeating these.
3835
seen_types: FxIndexSet<Type>,
3936
seen_consts: FxIndexSet<Const>,
40-
seen_data_inst_forms: FxIndexSet<DataInstForm>,
4137
seen_global_vars: FxIndexSet<GlobalVar>,
4238
seen_funcs: FxIndexSet<Func>,
4339
}
@@ -57,11 +53,6 @@ impl Visitor<'_> for ReachableUseCollector<'_> {
5753
self.visit_const_def(&self.cx[ct]);
5854
}
5955
}
60-
fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) {
61-
if self.seen_data_inst_forms.insert(data_inst_form) {
62-
self.visit_data_inst_form_def(&self.cx[data_inst_form]);
63-
}
64-
}
6556

6657
fn visit_global_var_use(&mut self, gv: GlobalVar) {
6758
if self.seen_global_vars.insert(gv) {

‎src/passes/link.rs‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::transform::{InnerTransform, Transformed, Transformer};
22
use crate::visit::{InnerVisit, Visitor};
33
use crate::{
4-
AttrSet, Const, Context, DataInstForm, DeclDef, ExportKey, Exportee, Func, FxIndexSet,
5-
GlobalVar, Import, Module, Type,
4+
AttrSet, Const, Context, DeclDef, ExportKey, Exportee, Func, FxIndexSet, GlobalVar, Import,
5+
Module, Type,
66
};
77
use rustc_hash::{FxHashMap, FxHashSet};
88
use std::collections::VecDeque;
@@ -33,7 +33,6 @@ pub fn minimize_exports(module: &mut Module, is_root: impl Fn(&ExportKey) -> boo
3333

3434
seen_types: FxHashSet::default(),
3535
seen_consts: FxHashSet::default(),
36-
seen_data_inst_forms: FxHashSet::default(),
3736
seen_global_vars: FxHashSet::default(),
3837
seen_funcs: FxHashSet::default(),
3938
};
@@ -61,7 +60,6 @@ struct LiveExportCollector<'a> {
6160
// FIXME(eddyb) build some automation to avoid ever repeating these.
6261
seen_types: FxHashSet<Type>,
6362
seen_consts: FxHashSet<Const>,
64-
seen_data_inst_forms: FxHashSet<DataInstForm>,
6563
seen_global_vars: FxHashSet<GlobalVar>,
6664
seen_funcs: FxHashSet<Func>,
6765
}
@@ -81,11 +79,6 @@ impl Visitor<'_> for LiveExportCollector<'_> {
8179
self.visit_const_def(&self.cx[ct]);
8280
}
8381
}
84-
fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) {
85-
if self.seen_data_inst_forms.insert(data_inst_form) {
86-
self.visit_data_inst_form_def(&self.cx[data_inst_form]);
87-
}
88-
}
8982

9083
fn visit_global_var_use(&mut self, gv: GlobalVar) {
9184
if self.seen_global_vars.insert(gv) {
@@ -128,7 +121,6 @@ pub fn resolve_imports(module: &mut Module) {
128121

129122
seen_types: FxHashSet::default(),
130123
seen_consts: FxHashSet::default(),
131-
seen_data_inst_forms: FxHashSet::default(),
132124
seen_global_vars: FxHashSet::default(),
133125
seen_funcs: FxHashSet::default(),
134126
};
@@ -144,7 +136,6 @@ pub fn resolve_imports(module: &mut Module) {
144136

145137
transformed_types: FxHashMap::default(),
146138
transformed_consts: FxHashMap::default(),
147-
transformed_data_inst_forms: FxHashMap::default(),
148139
transformed_global_vars: FxHashMap::default(),
149140
global_var_queue: VecDeque::new(),
150141
transformed_funcs: FxHashMap::default(),
@@ -179,7 +170,6 @@ struct ImportResolutionCollector<'a> {
179170
// FIXME(eddyb) build some automation to avoid ever repeating these.
180171
seen_types: FxHashSet<Type>,
181172
seen_consts: FxHashSet<Const>,
182-
seen_data_inst_forms: FxHashSet<DataInstForm>,
183173
seen_global_vars: FxHashSet<GlobalVar>,
184174
seen_funcs: FxHashSet<Func>,
185175
}
@@ -199,11 +189,6 @@ impl Visitor<'_> for ImportResolutionCollector<'_> {
199189
self.visit_const_def(&self.cx[ct]);
200190
}
201191
}
202-
fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) {
203-
if self.seen_data_inst_forms.insert(data_inst_form) {
204-
self.visit_data_inst_form_def(&self.cx[data_inst_form]);
205-
}
206-
}
207192

208193
fn visit_global_var_use(&mut self, gv: GlobalVar) {
209194
if self.seen_global_vars.insert(gv) {
@@ -250,7 +235,6 @@ struct ImportResolver<'a> {
250235
// FIXME(eddyb) build some automation to avoid ever repeating these.
251236
transformed_types: FxHashMap<Type, Transformed<Type>>,
252237
transformed_consts: FxHashMap<Const, Transformed<Const>>,
253-
transformed_data_inst_forms: FxHashMap<DataInstForm, Transformed<DataInstForm>>,
254238
transformed_global_vars: FxHashMap<GlobalVar, Transformed<GlobalVar>>,
255239
global_var_queue: VecDeque<GlobalVar>,
256240
transformed_funcs: FxHashMap<Func, Transformed<Func>>,
@@ -277,19 +261,6 @@ impl Transformer for ImportResolver<'_> {
277261
self.transformed_consts.insert(ct, transformed);
278262
transformed
279263
}
280-
fn transform_data_inst_form_use(
281-
&mut self,
282-
data_inst_form: DataInstForm,
283-
) -> Transformed<DataInstForm> {
284-
if let Some(&cached) = self.transformed_data_inst_forms.get(&data_inst_form) {
285-
return cached;
286-
}
287-
let transformed = self
288-
.transform_data_inst_form_def(&self.cx[data_inst_form])
289-
.map(|data_inst_form_def| self.cx.intern(data_inst_form_def));
290-
self.transformed_data_inst_forms.insert(data_inst_form, transformed);
291-
transformed
292-
}
293264

294265
fn transform_global_var_use(&mut self, gv: GlobalVar) -> Transformed<GlobalVar> {
295266
if let Some(&cached) = self.transformed_global_vars.get(&gv) {

‎src/passes/qptr.rs‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
//! [`QPtr`](crate::TypeKind::QPtr) transforms.
22
3+
use crate::qptr;
34
use crate::visit::{InnerVisit, Visitor};
45
use crate::{AttrSet, Const, Context, Func, FxIndexSet, GlobalVar, Module, Type};
5-
use crate::{DataInstForm, qptr};
66

77
pub fn lower_from_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConfig) {
88
let cx = &module.cx();
@@ -15,7 +15,6 @@ pub fn lower_from_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConf
1515

1616
seen_types: FxIndexSet::default(),
1717
seen_consts: FxIndexSet::default(),
18-
seen_data_inst_forms: FxIndexSet::default(),
1918
seen_global_vars: FxIndexSet::default(),
2019
seen_funcs: FxIndexSet::default(),
2120
};
@@ -50,7 +49,6 @@ pub fn lift_to_spv_ptrs(module: &mut Module, layout_config: &qptr::LayoutConfig)
5049

5150
seen_types: FxIndexSet::default(),
5251
seen_consts: FxIndexSet::default(),
53-
seen_data_inst_forms: FxIndexSet::default(),
5452
seen_global_vars: FxIndexSet::default(),
5553
seen_funcs: FxIndexSet::default(),
5654
};
@@ -75,7 +73,6 @@ struct ReachableUseCollector<'a> {
7573
// FIXME(eddyb) build some automation to avoid ever repeating these.
7674
seen_types: FxIndexSet<Type>,
7775
seen_consts: FxIndexSet<Const>,
78-
seen_data_inst_forms: FxIndexSet<DataInstForm>,
7976
seen_global_vars: FxIndexSet<GlobalVar>,
8077
seen_funcs: FxIndexSet<Func>,
8178
}
@@ -95,11 +92,6 @@ impl Visitor<'_> for ReachableUseCollector<'_> {
9592
self.visit_const_def(&self.cx[ct]);
9693
}
9794
}
98-
fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) {
99-
if self.seen_data_inst_forms.insert(data_inst_form) {
100-
self.visit_data_inst_form_def(&self.cx[data_inst_form]);
101-
}
102-
}
10395

10496
fn visit_global_var_use(&mut self, gv: GlobalVar) {
10597
if self.seen_global_vars.insert(gv) {

‎src/print/mod.rs‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ use crate::qptr::{self, QPtrAttr, QPtrMemUsage, QPtrMemUsageKind, QPtrOp, QPtrUs
2525
use crate::visit::{InnerVisit, Visit, Visitor};
2626
use crate::{
2727
AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstDef, ConstKind, Context, DataInst,
28-
DataInstDef, DataInstForm, DataInstFormDef, DataInstKind, DbgSrcLoc, DeclDef, Diag, DiagLevel,
29-
DiagMsgPart, EntityListIter, ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap,
30-
FxIndexSet, GlobalVar, GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo,
31-
ModuleDialect, Node, NodeDef, NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef,
32-
RegionInputDecl, SelectionKind, Type, TypeDef, TypeKind, TypeOrConst, Value, cfg, spv,
28+
DataInstDef, DataInstKind, DbgSrcLoc, DeclDef, Diag, DiagLevel, DiagMsgPart, EntityListIter,
29+
ExportKey, Exportee, Func, FuncDecl, FuncParam, FxIndexMap, FxIndexSet, GlobalVar,
30+
GlobalVarDecl, GlobalVarDefBody, Import, Module, ModuleDebugInfo, ModuleDialect, Node, NodeDef,
31+
NodeKind, NodeOutputDecl, OrdAssertEq, Region, RegionDef, RegionInputDecl, SelectionKind, Type,
32+
TypeDef, TypeKind, TypeOrConst, Value, cfg, spv,
3333
};
3434
use arrayvec::ArrayVec;
3535
use itertools::Either;
@@ -463,12 +463,6 @@ impl<'a> Visitor<'a> for Plan<'a> {
463463
fn visit_const_use(&mut self, ct: Const) {
464464
self.use_interned(CxInterned::Const(ct));
465465
}
466-
fn visit_data_inst_form_use(&mut self, data_inst_form: DataInstForm) {
467-
// NOTE(eddyb) this contains no deduplication because each `DataInstDef`
468-
// will be pretty-printed separately, so everything in its `form` also
469-
// needs to get use counts incremented separately per-`DataInstDef`.
470-
self.visit_data_inst_form_def(&self.cx[data_inst_form]);
471-
}
472466

473467
fn visit_global_var_use(&mut self, gv: GlobalVar) {
474468
if let Some(module) = self.current_module {
@@ -974,7 +968,7 @@ impl<'a> Printer<'a> {
974968
None,
975969
);
976970
let inst_def = func_at_inst.def();
977-
if cx[inst_def.form].output_type.is_some() {
971+
if inst_def.output_type.is_some() {
978972
define(
979973
Use::DataInstOutput(func_at_inst.position),
980974
Some(inst_def.attrs),
@@ -1000,7 +994,6 @@ impl<'a> Printer<'a> {
1000994
fn visit_attr_set_use(&mut self, _: AttrSet) {}
1001995
fn visit_type_use(&mut self, _: Type) {}
1002996
fn visit_const_use(&mut self, _: Const) {}
1003-
fn visit_data_inst_form_use(&mut self, _: DataInstForm) {}
1004997
fn visit_global_var_use(&mut self, _: GlobalVar) {}
1005998
fn visit_func_use(&mut self, _: Func) {}
1006999

@@ -3153,12 +3146,10 @@ impl Print for NodeOutputDecl {
31533146
impl Print for FuncAt<'_, DataInst> {
31543147
type Output = pretty::Fragment;
31553148
fn print(&self, printer: &Printer<'_>) -> pretty::Fragment {
3156-
let DataInstDef { attrs, form, inputs } = self.def();
3149+
let DataInstDef { attrs, kind, inputs, output_type } = self.def();
31573150

31583151
let attrs = attrs.print(printer);
31593152

3160-
let DataInstFormDef { kind, output_type } = &printer.cx[*form];
3161-
31623153
let mut output_use_to_print_as_lhs =
31633154
output_type.map(|_| Use::DataInstOutput(self.position));
31643155

‎src/qptr/analyze.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use super::{QPtrAttr, QPtrMemUsage, QPtrOp, QPtrUsage, shapes};
77
use crate::func_at::FuncAt;
88
use crate::visit::{InnerVisit, Visitor};
99
use crate::{
10-
AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, DataInst, DataInstForm,
11-
DataInstKind, DeclDef, Diag, EntityList, ExportKey, Exportee, Func, FxIndexMap, GlobalVar,
12-
Module, Node, NodeKind, OrdAssertEq, Type, TypeKind, Value,
10+
AddrSpace, Attr, AttrSet, AttrSetDef, Const, ConstKind, Context, DataInst, DataInstKind,
11+
DeclDef, Diag, EntityList, ExportKey, Exportee, Func, FxIndexMap, GlobalVar, Module, Node,
12+
NodeKind, OrdAssertEq, Type, TypeKind, Value,
1313
};
1414
use itertools::Either;
1515
use rustc_hash::FxHashMap;
@@ -866,7 +866,6 @@ impl<'a> InferUsage<'a> {
866866
for func_at_inst in func_def_body.at(insts).into_iter().rev() {
867867
let data_inst = func_at_inst.position;
868868
let data_inst_def = func_at_inst.def();
869-
let data_inst_form_def = &cx[data_inst_def.form];
870869
let output_usage = data_inst_output_usages.remove(&data_inst).flatten();
871870

872871
let mut generate_usage = |this: &mut Self, ptr: Value, new_usage| {
@@ -904,7 +903,7 @@ impl<'a> InferUsage<'a> {
904903
None => new_usage,
905904
});
906905
};
907-
match &data_inst_form_def.kind {
906+
match &data_inst_def.kind {
908907
&DataInstKind::FuncCall(callee) => {
909908
match self.infer_usage_in_func(module, callee) {
910909
FuncInferUsageState::Complete(callee_results) => {
@@ -925,7 +924,7 @@ impl<'a> InferUsage<'a> {
925924
));
926925
}
927926
};
928-
if data_inst_form_def.output_type.map_or(false, is_qptr) {
927+
if data_inst_def.output_type.map_or(false, is_qptr) {
929928
if let Some(usage) = output_usage {
930929
usage_or_err_attrs_to_attach
931930
.push((Value::DataInstOutput(data_inst), usage));
@@ -1108,7 +1107,7 @@ impl<'a> InferUsage<'a> {
11081107
}
11091108
DataInstKind::QPtr(op @ (QPtrOp::Load | QPtrOp::Store)) => {
11101109
let (op_name, access_type) = match op {
1111-
QPtrOp::Load => ("Load", data_inst_form_def.output_type.unwrap()),
1110+
QPtrOp::Load => ("Load", data_inst_def.output_type.unwrap()),
11121111
QPtrOp::Store => {
11131112
("Store", func_at_inst.at(data_inst_def.inputs[1]).type_of(&cx))
11141113
}
@@ -1250,7 +1249,6 @@ impl Visitor<'_> for CollectAllDataInsts {
12501249
fn visit_attr_set_use(&mut self, _: AttrSet) {}
12511250
fn visit_type_use(&mut self, _: Type) {}
12521251
fn visit_const_use(&mut self, _: Const) {}
1253-
fn visit_data_inst_form_use(&mut self, _: DataInstForm) {}
12541252
fn visit_global_var_use(&mut self, _: GlobalVar) {}
12551253
fn visit_func_use(&mut self, _: Func) {}
12561254

0 commit comments

Comments
 (0)