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

IR Pass based #14

Closed
wants to merge 25 commits into from
Closed

IR Pass based #14

wants to merge 25 commits into from

Conversation

oxinabox
Copy link
Owner

To be a Real debugger,
you need to be able to step though a method,
call by call, seeing how all the local variables are changing.

Not just our current game of Leap to the next call and examine it's inputs.
(and with #5 outputs)

This PR is to be able to do that, or at least the core parts of that

src/detailed.jl Outdated Show resolved Hide resolved
@jrevels
Copy link

jrevels commented Jan 25, 2019

So cool :)

@jrevels
Copy link

jrevels commented Jan 25, 2019

This is actually kind of interesting. @vtjnash + @Keno + me + others have been musing for a while about what it would look like for pre-type-inference IR to be fully SSA form (like the optimizer's IR). If that were to happen, you couldn't accomplish the thing you've implemented here via a simple Cassette pass. Though I guess we could bake-in additional mechanisms to ensure this kind of thing could still work (e.g. the compiler could keep around a separate queryable cache for the original variable names etc.). This change isn't likely to happen in the near future (or maybe ever) anyway, just food for thought.

@vtjnash
Copy link

vtjnash commented Jan 25, 2019

FWIW, I don't think we should ever delete the ability to handle Slots from inference. While SSA form is helpful for writing mutation passes (it simplifies updating non-local information), it's not always a clear win (there's a reason that clang avoids them). So don't avoid them just because the later optimizer passes don't need to optimize them, or think that means we need to remove them from the IR.

@jrevels
Copy link

jrevels commented Jan 25, 2019

FWIW, I don't think we should ever delete the ability to handle Slots from inference. While SSA form is helpful for writing mutation passes (it simplifies updating non-local information), it's not always a clear win (there's a reason that clang avoids them). So don't avoid them just because the later optimizer passes don't need to optimize them, or think that means we need to remove them from the IR.

Certainly, not advocating for removing slots. Just pointing out that this PR demonstrates one of the potential negative consequences of removing them (it'd make this PR harder to implement) 🙂

@oxinabox
Copy link
Owner Author

With a bit more tinkering it works!

julia-1.1 --project=. test/test_assignment_capture.jl
Dict{Any,Any} with 3 entries:
  Symbol("##eg_last#381") => Dict{Any,Any}()
  :z => 6
  Symbol("##eg2#379") => Dict{Any,Any}(:y=>5,Symbol("##+#378")=>Dict{Any,Any}(),Symbol("##eg21#372")=>Dict{Any,Any}(Symbol("##*#371")=>Dict{Any,Any}()),Symbol("##eg3#377")=>Dict{Any,Any}(Symbol("##+#376")=>Dict{Any,Any}()),:ret=>6)

@jrevels do you think this might be a nice example for the Cassette docs?
It is a lot shorter and I think might be simpler than the sliceprintln thing

@oxinabox
Copy link
Owner Author

oxinabox commented Mar 3, 2019

All the tests are probably broken now.
But this now has the core functionality (and even User Commands working) around being able to break/step midway through a method.
This is done by inserting an extra call in between every statement.

Todo is to hook the breakpoint stuff in (right now it just initially begins by breakpointing immediately),
then to add some polish by not recompiling literally everything and adding in this code.
This can be done by not recurseing calls that we are going to StepNext over; though the problem there is what if it called a breakpointed method.
So probably actually need to be a bit smarter than that, but that is the principle.

@oxinabox oxinabox changed the title [WIP] Be able to look while you leap -- capturing local variables IR Pass based Mar 5, 2019
@oxinabox oxinabox mentioned this pull request Mar 9, 2019
@oxinabox
Copy link
Owner Author

oxinabox commented Mar 9, 2019

Merged into #23

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 this pull request may close these issues.

3 participants