Skip to content

Alloc remapping #2

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.

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

Merged
merged 6 commits into from
Jun 26, 2024
Merged

Alloc remapping #2

merged 6 commits into from
Jun 26, 2024

Conversation

dkcumming
Copy link
Collaborator

Interning of AllocId in serialisation of SMIR, and then separate serialisation of GlobalAlloc mapping to interned values.

@dkcumming dkcumming requested a review from sskeirik June 26, 2024 05:13
@dkcumming dkcumming self-assigned this Jun 26, 2024
Comment on lines +51 to +72
fn get_index_and_populate_allocs(scc: &mut SerializeCycleCheck, alloc_id: &AllocId) -> usize {
let galloc = &GlobalAlloc::from(*alloc_id);
if !scc.seen_allocs.contains(alloc_id) {
scc.seen_allocs.insert(*alloc_id);
scc.gallocs_ordered.push(galloc.clone());
match galloc {
GlobalAlloc::Memory(allocation) => {
(&allocation.provenance.ptrs)
.into_iter()
.for_each(|(_, prov)| { get_index_and_populate_allocs(scc, &prov.0); })
},
_ => {},
}
scc.seen_allocs.len() - 1
} else {
(&scc.gallocs_ordered)
.into_iter()
.position(|needle| galloc == needle)
.unwrap()
}

}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not the best way to handle things, I feel like there is a lot of cloning especially since the whole vector gets cloned in global_allocs function. What do you think @sskeirik?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, let's not worry about this. We can always fix it in a later version.

Copy link
Collaborator

@sskeirik sskeirik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for a first pass. Let's optimize later.

Comment on lines +51 to +72
fn get_index_and_populate_allocs(scc: &mut SerializeCycleCheck, alloc_id: &AllocId) -> usize {
let galloc = &GlobalAlloc::from(*alloc_id);
if !scc.seen_allocs.contains(alloc_id) {
scc.seen_allocs.insert(*alloc_id);
scc.gallocs_ordered.push(galloc.clone());
match galloc {
GlobalAlloc::Memory(allocation) => {
(&allocation.provenance.ptrs)
.into_iter()
.for_each(|(_, prov)| { get_index_and_populate_allocs(scc, &prov.0); })
},
_ => {},
}
scc.seen_allocs.len() - 1
} else {
(&scc.gallocs_ordered)
.into_iter()
.position(|needle| galloc == needle)
.unwrap()
}

}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, let's not worry about this. We can always fix it in a later version.

@dkcumming dkcumming merged commit c31a204 into smir_serde Jun 26, 2024
@dkcumming dkcumming deleted the alloc_remapping branch June 26, 2024 23:48
sskeirik pushed a commit that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants