Refactor compiler method lookup interface#36743
Merged
Conversation
Member
Author
|
Realized |
| @@ -7,8 +7,7 @@ to re-consult the method table. This info is illegal on any statement that is | |||
| not a call to a generic function. | |||
| """ | |||
| struct MethodMatchInfo | |||
Member
There was a problem hiding this comment.
Maybe this type should be combined with MethodLookupResult?
Member
Author
There was a problem hiding this comment.
I'm not quite sure yet. There's some additional information about method matches I may want to keep in the near future, that I think could go here nicely. My preference would be to keep this as is for now and if it turns out that information would be better somewhere else, we can always refactor this, since it's all internal.
The primary motivation here is to clean up the notion of "looking up a method in the method table" into a single object that can be passed around. At the moment, it's all a bit murky, with fairly large state objects being passed around everywhere and implicit accesses to the global environment. In my AD use case, I need to be a bit careful to make sure the various inference and optimization steps are looking things up in the correct tables/caches, so being very explicit about where things need to be looked up is quite helpful. In particular, I would like to clean up the optimizer, to not require the big `OptimizationState` which is currently a bit of a mix of things that go into IRCode and information needed for method lookup/edge tracking. That isn't part of this PR, but will build on top of it. More generally, with a bunch of the recent compiler work, I've been trying to define more crisp boundaries between the various components of the system, giving them clearer interfaces, and at least a little bit of documentation. The compiler is a very powerful bit of technology, but I think people having been avoiding it, because the code looks a bit scary. I'm hoping some of these cleanups will make it easier for people to understand what's going on. Here in particular, I'm using `findall(sig, table)` as the predicate for method lookup. The idea being that people familiar with the `findall(predicate, collection)` idiom from regular julia will have a good intuitive understanding of what's happening (a collection is searched for a predicate), an array of matches is returned, etc. Of course, it's not a perfect fit, but I think these kinds of mental aids can be helpful in making it easier for people to read compiler code (similar to how #35831 used `getindex` as the verb for cache lookup). While I was at it, I also cleaned up the use of out-parameters which leaked through too much of the underlying C API and replaced them by a proper struct of results.
Keno
added a commit
that referenced
this pull request
Jul 23, 2020
This was accidentally lost in #36743 while I was shuffling aroudn what these methods actually return. However, we also didn't have a test to catch that, so this adds such a test as well.
Keno
added a commit
that referenced
this pull request
Jul 23, 2020
This was accidentally lost in #36743 while I was shuffling aroudn what these methods actually return. However, we also didn't have a test to catch that, so this adds such a test as well.
simeonschaub
pushed a commit
to simeonschaub/julia
that referenced
this pull request
Aug 11, 2020
The primary motivation here is to clean up the notion of "looking up a method in the method table" into a single object that can be passed around. At the moment, it's all a bit murky, with fairly large state objects being passed around everywhere and implicit accesses to the global environment. In my AD use case, I need to be a bit careful to make sure the various inference and optimization steps are looking things up in the correct tables/caches, so being very explicit about where things need to be looked up is quite helpful. In particular, I would like to clean up the optimizer, to not require the big `OptimizationState` which is currently a bit of a mix of things that go into IRCode and information needed for method lookup/edge tracking. That isn't part of this PR, but will build on top of it. More generally, with a bunch of the recent compiler work, I've been trying to define more crisp boundaries between the various components of the system, giving them clearer interfaces, and at least a little bit of documentation. The compiler is a very powerful bit of technology, but I think people having been avoiding it, because the code looks a bit scary. I'm hoping some of these cleanups will make it easier for people to understand what's going on. Here in particular, I'm using `findall(sig, table)` as the predicate for method lookup. The idea being that people familiar with the `findall(predicate, collection)` idiom from regular julia will have a good intuitive understanding of what's happening (a collection is searched for a predicate), an array of matches is returned, etc. Of course, it's not a perfect fit, but I think these kinds of mental aids can be helpful in making it easier for people to read compiler code (similar to how JuliaLang#35831 used `getindex` as the verb for cache lookup). While I was at it, I also cleaned up the use of out-parameters which leaked through too much of the underlying C API and replaced them by a proper struct of results.
simeonschaub
pushed a commit
to simeonschaub/julia
that referenced
this pull request
Aug 11, 2020
This was accidentally lost in JuliaLang#36743 while I was shuffling aroudn what these methods actually return. However, we also didn't have a test to catch that, so this adds such a test as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The primary motivation here is to clean up the notion of
"looking up a method in the method table" into a single
object that can be passed around. At the moment, it's
all a bit murky, with fairly large state objects being
passed around everywhere and implicit accesses to the
global environment. In my AD use case, I need to be a
bit careful to make sure the various inference and
optimization steps are looking things up in the correct
tables/caches, so being very explicit about where things
need to be looked up is quite helpful.
In particular, I would like to clean up the optimizer, to
not require the big
OptimizationStatewhich is currentlya bit of a mix of things that go into IRCode and information
needed for method lookup/edge tracking. That isn't part of
this PR, but will build on top of it.
More generally, with a bunch of the recent compiler work,
I've been trying to define more crisp boundaries between
the various components of the system, giving them clearer
interfaces, and at least a little bit of documentation.
The compiler is a very powerful bit of technology, but I
think people having been avoiding it, because the code
looks a bit scary. I'm hoping some of these cleanups will
make it easier for people to understand what's going on.
Here in particular, I'm using
findall(sig, table)asthe predicate for method lookup. The idea being that
people familiar with the
findall(predicate, collection)idiom from regular julia will have a good intuitive
understanding of what's happening (a collection is searched
for a predicate), an array of matches is returned, etc.
Of course, it's not a perfect fit, but I think these
kinds of mental aids can be helpful in making it easier
for people to read compiler code (similar to how #35831
used
getindexas the verb for cache lookup). WhileI was at it, I also cleaned up the use of out-parameters
which leaked through too much of the underlying C API
and replaced them by a proper struct of results.