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

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

Projects

None yet

3 participants

@subbuss
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
Contributor
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
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
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
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
Contributor
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
Contributor
subbuss commented Sep 21, 2016

Created #4167.

@subbuss subbuss closed this 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