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

AddCallProtocol unconditionally forces frame #1456

Closed
headius opened this issue Jan 28, 2014 · 4 comments
Closed

AddCallProtocol unconditionally forces frame #1456

headius opened this issue Jan 28, 2014 · 4 comments
Labels

Comments

@headius
Copy link
Member

@headius headius commented Jan 28, 2014

I see that push/pop of binding are omitted when local variables are not needed across calls, but push/pop of frame appear to be emitted all the time:

For code "def foo(a); p a; end" we get the following instrs:

  0 push_frame
  1 check_arity(1, 0, -1)
  2 %t_a_1 = recv_pre_reqd_arg(0)
  3 thread_poll
  4 line_num(0)
  5 %v_0 = call_1o(FUNCTIONAL, 'p', %self, [%t_a_1]){1O}
  6 pop_frame
  7 return(%v_0)
  8 [DEAD]%v_3 = recv_jruby_exc
  9 [DEAD]pop_frame
  10    [DEAD]throw(%v_3)

I guess this reflects a lack of awareness about frame fields in IR right now.

/cc @subbuss @enebo

@subbuss
Copy link
Contributor

@subbuss subbuss commented Jan 28, 2014

That is correct. IRScope needs to add a needsFrame flag and set it based on whatever heuristics are currently used in AST JIT.

@enebo
Copy link
Member

@enebo enebo commented Jan 28, 2014

This is silly but:

%frame = push_frame

pop_frame %frame

Could be made to work naturally without needing to add a frame but that
might be a bit too much work to not add another flag to IRScope

-Tom

On Tue, Jan 28, 2014 at 9:43 AM, Charles Oliver Nutter <
notifications@github.com> wrote:

I see that push/pop of binding are omitted when local variables are not
needed across calls, but push/pop of frame appear to be emitted all the
time:

For code "def foo(a); p a; end" we get the following instrs:

0 push_frame
1 check_arity(1, 0, -1)
2 %t_a_1 = recv_pre_reqd_arg(0)
3 thread_poll
4 line_num(0)
5 %v_0 = call_1o(FUNCTIONAL, 'p', %self, [%t_a_1]){1O}
6 pop_frame
7 return(%v_0)
8 [DEAD]%v_3 = recv_jruby_exc
9 [DEAD]pop_frame
10 [DEAD]throw(%v_3)

I guess this reflects a lack of awareness about frame fields in IR right
now.

/cc @subbuss https://github.com/subbuss @enebohttps://github.com/enebo

Reply to this email directly or view it on GitHubhttps://github.com//issues/1456
.

blog: http://blog.enebo.com twitter: tom_enebo
mail: tom.enebo@gmail.com

@enebo
Copy link
Member

@enebo enebo commented Jan 29, 2014

I should have replied to this instead of an email thread on jruby mailing list...Just compile all dead instructions until this is resolved since compiling dead instructions will not affect programs correctness. It might also stress the JIT more anyways as well.

@enebo
Copy link
Member

@enebo enebo commented Jan 15, 2015

Fixed a long time ago.

@enebo enebo closed this as completed Jan 15, 2015
@headius headius removed this from the JRuby 9.0.0.0 milestone Jan 16, 2015
@headius headius added this to the JRuby 9.0.0.0-pre1 milestone Jan 16, 2015
@headius headius added this to the JRuby 9.0.0.0-pre1 milestone Jan 16, 2015
@headius headius removed this from the JRuby 9.0.0.0 milestone Jan 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants