Skip to content
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

merge allocations in same basic block up to a size bound #1125

Merged
merged 8 commits into from
Aug 5, 2024

Conversation

dwightguth
Copy link
Collaborator

Previously, when we were allocating a large term, we would issue a number of repeated calls to kore_alloc. This is inefficient because it has to check whether there is enough space and execute a branch instruction on each allocation. It is more efficient if we do one, larger call to kore_alloc for all the memory that we need.

In order to efficiently implement this, we modify the way we generate calls to kore_alloc in the code generator so that it will place the allocation function at the beginning of the basic block and then, if further allocations are required in the same basic block, it will modify the call to the allocation function so it requests a larger amount of memory. This only happens up to a certain bound, in order to prevent memory from being wasted due to requesting particularly large chunks of memory. In practice, this leads to most apply_rule functions only calling kore_alloc once.

@rv-jenkins rv-jenkins changed the base branch from master to develop August 1, 2024 18:59
@dwightguth dwightguth marked this pull request as ready for review August 2, 2024 17:45
Copy link
Collaborator

@Robertorosmaninho Robertorosmaninho left a comment

Choose a reason for hiding this comment

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

The PR LGTM! However, there's a missing debug info that we're not emitting in the new function that I would like to keep the old behavior for consistence.

include/kllvm/codegen/CreateTerm.h Outdated Show resolved Hide resolved
lib/codegen/CreateTerm.cpp Show resolved Hide resolved
Copy link
Collaborator

@Robertorosmaninho Robertorosmaninho left a comment

Choose a reason for hiding this comment

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

Besides my last comment about debug info, it looks good to me!

@dwightguth dwightguth enabled auto-merge (squash) August 5, 2024 14:55
@dwightguth dwightguth merged commit 9088cef into develop Aug 5, 2024
10 checks passed
@dwightguth dwightguth deleted the merge-allocs branch August 5, 2024 15:37
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