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.

Sign up for GitHub

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

Jump to bottom

Add new make_fun3 instruction #2765

Closed
wants to merge 0 commits into from
Closed

Add new make_fun3 instruction #2765

wants to merge 0 commits into from

Conversation

sverker
Copy link
Contributor

@sverker sverker commented Sep 22, 2020 •
edited
Loading

*** MERGED *** MERGED *** MERGED ***
This PR is merged to master at 1073fa2

Compared to old make_fun2:

  • No GC done.
  • Environment and destination are arguments of instruction and not assumed x-registers.

Results in more effective register allocations.

@sverker sverker self-assigned this Sep 22, 2020
@sverker sverker added enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Sep 22, 2020
@bjorng
Copy link
Contributor

bjorng commented Sep 23, 2020

Looks good to me.

I have only one minor nit to pick: For Co-authored-by to work as intended, it must be placed last in the commit message.

bjorng
bjorng reviewed Oct 5, 2020
View reviewed changes
lib/compiler/src/genop.tab Outdated
@@ -608,3 +608,5 @@ BEAM_FORMAT_NUMBER=0
## it will never fail at runtime, or 'resume' when we know the input is
## a match context.
170: bs_start_match4/4

171: make_fun3/3
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document make_fun/3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@bjorng
Copy link
Contributor

bjorng commented Oct 5, 2020

@sverker Two test cases fail in the compiler test suite on host ecthelion (to recreate locally, give the cover option when runnning the test suites).

You will need to include no_make_fun3 must be included in test_lib:opt_opts/1.

@garazdawi garazdawi added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Oct 6, 2020
@sverker sverker closed this Oct 7, 2020
@sverker
Copy link
Contributor Author

sverker commented Oct 7, 2020

*** MERGED *** MERGED *** MERGED ***
This PR is merged to master at 1073fa2

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

@bjorng bjorng bjorng left review comments

Assignees

@sverker sverker

Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sverker @bjorng @garazdawi