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

Build a new pass that ensures that ruby locals / IR tmps are initialized #2465

Closed
subbuss opened this issue Jan 15, 2015 · 6 comments
Closed

Build a new pass that ensures that ruby locals / IR tmps are initialized #2465

subbuss opened this issue Jan 15, 2015 · 6 comments
Labels
ir
Milestone
JRuby 9.1.6.0

Comments

@subbuss
Copy link
Contributor

subbuss commented Jan 15, 2015

So, in the snippet below, x is not initialized on all paths.

def foo
  x = 1 if bar
  p x
end

So, this requires JRuby to return nil if a local var look fails. We can get rid of those if the IR can introduce necessarily initializations on all code paths. This will simply DynamicScope var looks and also eliminate conditionals in variable retrieves (see https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/ir/operands/TemporaryLocalVariable.java#L48-L61)
@subbuss subbuss added the ir label Jan 15, 2015
@subbuss
Copy link
Contributor Author

subbuss commented Sep 20, 2016

I added the AddMissingInitsPass for this. However, we are not ready to get rid of the null check in DynamicScope var lookups since we have the interpreter that runs in startup mode where no passes have run. So, that would require a specialized implementation of DynamicScope for the JIT and the interpreter where the former would be vastly simplified without the null checks. That is a different beast and I am inclined to close this ticket and maybe a different ticket for using an optimized version of DynamicScope without null checks? /cc @headius @enebo ..

@headius
Copy link
Member

headius commented Sep 21, 2016

@subbuss That does sound like a good idea. If Full/JIT could allocate DynamicScope instances that don't do need to do null checks, it would help reduce code size and complexity, potentially letting more stuff inline and optimize. This could also be combined with dynamically generating DynamicScope subclasses that have exactly the right number of variables, so we're never allocating an array to hold them.

So yes, 👍 to opening a new issue for smarter DynamicScope null checking.

@enebo
Copy link
Member

enebo commented Sep 21, 2016

@subbuss @headius we could make dynamicscope only for startup interp and have default not null check. The only requirement is full interp would need to run this pass as well. So I am advocating removing the checks on current type and adding special one only for startup as the oddball.

@enebo
Copy link
Member

enebo commented Sep 21, 2016

Err I should add why I want this as well. If we want to test all passes usable and testable via full interp then I think we need to have full interp act as closely to JIT as we can internally.

@subbuss
Copy link
Contributor Author

subbuss commented Sep 21, 2016 •
edited

@enebo sure makes sense ... that the startup interp should be the exceptional case. @headius, will create a new issue later today and close this one.

@subbuss
Copy link
Contributor Author

subbuss commented Sep 21, 2016

Created #4167.

@subbuss subbuss closed this as completed Sep 21, 2016
@enebo enebo added this to the JRuby 9.1.6.0 milestone Nov 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ir
Projects
None yet
Milestone
JRuby 9.1.6.0
Development

No branches or pull requests
3 participants
@headius @enebo @subbuss