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

global variable woe #54

Closed
aviatesk opened this issue Oct 12, 2020 · 0 comments · Fixed by #56
Closed

global variable woe #54

aviatesk opened this issue Oct 12, 2020 · 0 comments · Fixed by #56

Comments

@aviatesk
Copy link
Owner

aviatesk commented Oct 12, 2020

There're mainly two big problems around how JET treats global variabels

1. propagate of non-constant global variables

I thought we can safely propagate the type of (non constant) global variable if we invalidate all the code cache that uses it upon its update (the force_invalidate! logic), but it was wrong.
This is just because we can't invalidate code cache if the update happens within the lineage frames of the current frame. Consider the following example:

var = '1'

function foo(v)
    rand(Bool) && setter!(v)
    sin(var) # <= var::Union{Char,Int} ?
end
setter!(v) = global var = v

foo(10)

Now we need to annotate var as Union{Char,Int}, but this annotation is actually very hard (or even impossible, with the existence of the widening heuristics) to be done in general case (while this particular example is very simple, setter! can happen in a deeply nested conditioned callee).

I guess this is why Julia's native compiler doesn't do what I had done for JET, and now I think I need to throw away the logic in JET as well; JET will just propagate Any for non-constant global variables.
Of course this will make JET's profiling less accurate, but it's consistent with Julia's JIT at least, and I feel we want to have performance-warnings for this instead.

2. control flows in toplevel frames

Well, as for toplevel frames, I rather want to keep VirtualGlobalVarible logic. Toplevel call signatures are really important for JET's profiling process, because they're "starting points" of our analysis.
Assuming assignments of global variables only happen in toplevel frames, I would like to improve the accuracy of our virtual global variable assignments.

Consider the following example:

v = rand(Int)
if rand(Bool)
    v = '1'
end

sin(v)

At sin(v), v should be annotated as v::Union{Int,Char} (and thus union-split NoMethodError should be reported), while JET now interprets toplevel code by per-basic-block basis, and only has really naive assignment logic, and so it's annotated as v::Char, which should be fixed anyway.

aviatesk added a commit that referenced this issue Oct 14, 2020
aviatesk added a commit that referenced this issue Oct 15, 2020
aviatesk added a commit that referenced this issue Oct 15, 2020
aviatesk added a commit that referenced this issue Oct 16, 2020
aviatesk added a commit that referenced this issue Oct 16, 2020
aviatesk added a commit that referenced this issue Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant