Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fix some things in the LLVM backend #319

Merged
merged 3 commits into from Apr 1, 2014

Conversation

Projects
None yet
4 participants
Contributor

yiannist commented Mar 30, 2014

This pull request fixes a few things in the LLVM backend.

In detail:

  • it handles the case when 'to_llvm' is used but a required LLVM version (3.4 or greater) doesn't appear in $PATH,
  • it fixes an error in the handling of closure names: in case of closures the actual arity is A (as of MFA) + 1, and
  • it fixes a nasty error in the computation of the frame size when setting up stack descriptors.

While at it, we re-worded some comments and did a minor cleanup.

yiannist and others added some commits Mar 26, 2014

Check for required LLVM version or issue error
This checks that a required LLVM version (i.e. 3.4 or greater) appears in $PATH
when 'to_llvm' flag is used; in case of failure, abort compilation with an
error.
Fix frame size adjustment of stack descriptors
In case of function calls with arguments that are passed to the stack,
the frame size of corresponding stack descriptors needs to be reduced by
the number of stack arguments. This commit fixes a bug in this
adjustment which was caused by an incorrect check.
Fix counting of arguments of closures
Do not rely on MFA name for the arity of functions, since closures have
an extra argument. Instead, just use the length of the arguments list.
Contributor

sverker commented Mar 31, 2014

Put in integration test for 17.0

Contributor

kostis commented Mar 31, 2014

If you graduate #307 soon and also install LLVM 3.4 on some x86_64 machine you will also get some actual testing of the new backend. I can also provide some more tests for it, which I've been holding on to till #307 is merged.

Contributor

sverker commented Mar 31, 2014

Thanks for the reminder

#307 now merged and pushed.

Contributor

kostis commented Mar 31, 2014

Thanks for the quick reaction. The tests that you currently have in lib/hipe/test should now be working both with vanilla HiPE and with the to_llvm option on, even without this pull request (#319) merged.

To run the additional tests that I will add/send with a new pull request, you will first need to merge this one (#319) , or you will get a seg fault in one of them.

@sverker sverker merged commit f03a239 into erlang:master Apr 1, 2014

@yiannist yiannist deleted the yiannist:erllvm-fixes branch Apr 1, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment