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

MCJIT initial support for PowerPC64 #3200

Closed
wants to merge 2 commits into from
Closed

MCJIT initial support for PowerPC64 #3200

wants to merge 2 commits into from

Conversation

gustavotemple
Copy link
Contributor

This adds a MCJIT initial support for PowerPC64.

gustavotemple and others added 2 commits November 10, 2014 12:43
This adds a MCJIT initial support for PowerPC64.
engine_->freeMachineCodeForFunction(function_);
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that this change currently leaks memory when applied? You should be able to use valgrind to check for memory leaks here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dbussink, thank you for the feedback, I will do that.

@yorickpeterse
Copy link
Contributor

Please rebase this branch based on the current branch, instead of merging. That way we don't end up with needless "Merge pull request ...." commits all over the place.

As an aside, if we're going to support mcjit we shouldn't add extra configure flags and conditional macros, it should instead be the JIT library (we're not going to run both the old and new setup in parallel).

@gustavotemple
Copy link
Contributor Author

@yorickpeterse, OK, I understand, thanks for the comment... In this case, is it OK if I change the macro from RBX_LLVM_MCJIT_ENABLED to something like RBX_LLVM_API_VER >= 306?

@dbussink
Copy link
Contributor

@gustavotemple Why not change it for all? Is there only a certain LLVM version that has the necessary MCJIT support? It would be great if we can switch to MCJIT for everything, but I tried in the past and it has a bunch of issues.

The biggest one is that we use a custom memory allocator so we can prevent memory growth over time of long living LLVM context objects. I couldn't find a way at that point to fix that, but maybe newer LLVM versions allow for solving this problem now.

@brixen
Copy link
Member

brixen commented Nov 26, 2014

@gustavotemple thanks for working on this. I want to move completely to the new API and stabilize with LLVM 3.5+, and I want to do this completely, not partially. I'm allocating time to work on it in the next couple weeks so I'm going to close this.

@brixen brixen closed this Nov 26, 2014
@gustavotemple
Copy link
Contributor Author

@brixen, OK, thank you

@gustavotemple
Copy link
Contributor Author

@dbussink, sorry for my late reply.
The LLVM 3.6 will be the first release without the old JIT.
I researched about the memory leak, I don't have sure about the memory leak in the MCJIT itself, but I think that the lib/CodeGen that the old JIT and the MCJIT uses, still have memory leak [1]

[1] http://llvm.org/bugs/show_bug.cgi?id=20124

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

4 participants