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.

Sign up for GitHub

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

Jump to bottom

inference: wrap constant-prop' result directly within OpaqueClosureCallInfo #42849

Merged
merged 2 commits into from
Oct 30, 2021
Merged

inference: wrap constant-prop' result directly within OpaqueClosureCallInfo #42849

merged 2 commits into from
Oct 30, 2021

Conversation

aviatesk
Copy link
Member

This is more consistent with the arrangement of InvokeCallInfo, and
will make succeeding processing a bit cleaner.

This change also avoids unnecessary specializations in callinfo constructions.

@aviatesk aviatesk added the merge me PR is reviewed. Merge when all tests are passing label Oct 29, 2021
@aviatesk
Copy link
Member Author

Rebased to take in #42841 .

@KristofferC
Copy link
Member

Feels a bit big to put into 1.7 at this point. Is it needed for something specific?

@aviatesk
inference: wrap constant-prop' result directly within `OpaqueClosureC…
bf1b5fa 
…allInfo`

This is more consistent with the arrangement of `InvokeCallInfo`, and
will make succeeding processing a bit cleaner.

I will make corresponding Cthulhu updates later.
@aviatesk
inference: avoid unnecessary specializations in callinfo constructions
4626cf6
@aviatesk
Copy link
Member Author

The main reason is that this change will eliminate unnecessary specializations of somewhat hot constructors, so it will help whole compilation run slightly more efficiently in general.

If we backport this, will we need to do extra experiments on compile-time latency / runtime performance ?
I'm almost sure this change won't hit any regression, but I'm just fine with no backport if you don't want to take the risk.

@aviatesk
Copy link
Member Author

aviatesk commented Oct 30, 2021 •
edited
Loading

Okay, let's not backport, I really don't want to increase the chance of delaying 1.7 release in anyway.

@KristofferC
Copy link
Member

We can always put it into 1.7.1. It is just in general good for things to be on master for a while before going into a release.

@aviatesk
Copy link
Member Author

I see, thanks for your explanation.

@aviatesk aviatesk merged commit a387724 into master Oct 30, 2021
@aviatesk aviatesk deleted the avi/callinfo branch October 30, 2021 08:41
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Oct 30, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
@aviatesk @LilithHafner
inference: wrap constant-prop' result directly within `OpaqueClosureC…
d4c96c5 
…allInfo` (JuliaLang#42849)

This is more consistent with the arrangement of `InvokeCallInfo`, and
will make succeeding processing a bit cleaner.

This change also avoids unnecessary specializations in callinfo constructions.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@aviatesk @LilithHafner
inference: wrap constant-prop' result directly within `OpaqueClosureC…
e061870 
…allInfo` (JuliaLang#42849)

This is more consistent with the arrangement of `InvokeCallInfo`, and
will make succeeding processing a bit cleaner.

This change also avoids unnecessary specializations in callinfo constructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash approved these changes

Assignees
No one assigned
Labels
compiler:inference Type inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aviatesk @KristofferC @vtjnash @DilumAluthge