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

Extract llvm_zone_t code into its own namespace. #398

Merged
merged 20 commits into from Feb 16, 2021

Conversation

nic-donaldson
Copy link
Contributor

Following a similar PR for closure_address_table, this PR moves llvm_zone_t into its own space.

This PR is carved out from this larger diff: master...nic-donaldson:llvm-refactor#files_bucket which upgrades to LLVM 11. This is part of separating code that includes or links with LLVM code from code that doesn't.

I made a couple of small changes in some functions while doing this, I'll call out those spots with comments because it's hard to spot code that has moved + changed.

src/EXTZones.cpp Outdated
free(Zone);
}

// NOMERGE: dropped 'inline' on lots of these functions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lots of the functions here had inline specifiers that I've removed. I don't have a great reason for this, but I think less code is better. The right thing to do here would be to measure the impact of this somehow but I don't have any plans to do that.

EXPORT void* llvm_zone_malloc(llvm_zone_t* zone, uint64_t size)
{
// NOMERGE: changed this
static std::unique_ptr<extemp::EXTMutex> alloc_mutex = []() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mutex was previously initialized somewhere else, but that meant it was polluting the global namespace a bit. I figure we can not do that and initialize it here where it's used instead.

I'm using unique_ptr here because MSVC didn't have the recursive_mutex(const recursive_mutex&) constructor, which is what this code tries to use if you write it without pointers.

@benswift
Copy link
Collaborator

Hey @nic-donaldson are you able to rebase this to the current master (55b47d7) and force push (which should update this PR)? That will re-run the tests (inc. on macOS 11) as well.

@nic-donaldson
Copy link
Contributor Author

Hey @nic-donaldson are you able to rebase this to the current master (55b47d7) and force push (which should update this PR)? That will re-run the tests (inc. on macOS 11) as well.

yep, done! let's hope they pass :|

@benswift benswift merged commit d1a2e87 into digego:master Feb 16, 2021
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.

None yet

2 participants