-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
inline invoke #9642
Closed
Closed
inline invoke #9642
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
b964e07
inline invoke
yuyichao 9a9862e
use TopNode
yuyichao 2d03d52
Try fix invoke inlining when the types parameter has side-effect. Alw…
yuyichao 5420562
typo in test....
yuyichao b0d5045
use jl_gf_invoke_lookup in typeinf to lookup the method to be called
yuyichao e68552d
do not throw error during typeinf if method matching failed
yuyichao 1646afe
generate direct call for invoke when all types are known at compile time
yuyichao f0d549b
do not embed the function to be invoke in the ast
yuyichao 95edd03
compile function
yuyichao 9196ca5
revert expr_type behavior
yuyichao e726434
jl_is_type already include jl_is_typevar
yuyichao 95d69c4
missing compile
yuyichao 1c8816b
comment and TODO about inlining invoke
yuyichao 91f4606
fix breakage
yuyichao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Submodule openspecfun
updated
from 381db9 to f3036c
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when inlining arguments, you need to go in reverse order to preserve the order-of-execution (see https://github.com/yuyichao/julia/blob/inline-invoke/base/inference.jl#L2770-L2771 for an example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is what I'm trying to do here. I've seen that part but I didn't really understand why. Isn't the arguments evaluated in the order they appears in the code?
Also the code generated seems to be currect here and that's why I didn't bother too much before sending the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm waiting for a build now, but what if you change the second argument to invoke to
(Any, Integer)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, i wasn't paying attention to the fact that you are always copying the argument to a temporary variable. there's no need to do that if the argument is
effect_free
/affect_free
(the difference linguistically is subtle: effect-free means that it does not cause an effect on surrounding code (e.g. pure), whereas affect-free means it is not affected by surrounding code (e.g. immutable))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type check is done after all arguments are evaluated so changing to
(Any, Integer)
does not affect the evaluation of the arguments at all. (which is the same schematics with callinginvoke
function)(Actually I think I might miss the case where evaluating the second argument to
invoke
has side effect)....There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, forgot to paste the output................