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

Add functions to support and use type hint in VP #16265

Merged
merged 1 commit into from
Dec 5, 2022
Merged

Add functions to support and use type hint in VP #16265

merged 1 commit into from
Dec 5, 2022

Conversation

a7ehuo
Copy link
Contributor

@a7ehuo a7ehuo commented Nov 4, 2022 •
edited
Loading

  • Add findLikelySubtype to look for a likely sub type given a TR_OpaqueClassBlock pointer or a class signature.
  • Add createTypeHintConstraint to create a constraint if a likely sub type for a given class signature is found
  • Create type hint for parameters in getParmValues
  • Create type hint using return type in innerConstrainAcall

Depends on

Co-Authored-By: Devin Papineau devin@ajdmp.ca
Signed-off-by: Annabelle Huo Annabelle.Huo@ibm.com

@a7ehuo a7ehuo added the comp:jit label Nov 4, 2022
@a7ehuo a7ehuo mentioned this pull request Nov 4, 2022
Merged
jdmpapin
jdmpapin reviewed Nov 11, 2022
View reviewed changes
Copy link
Contributor

@jdmpapin jdmpapin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind removing the blank line after the co-authored-by line in the commit message? GitHub only picks up metadata like that when there are no blank lines afterward

runtime/compiler/optimizer/J9ValuePropagation.cpp Outdated Show resolved Hide resolved
runtime/compiler/optimizer/J9ValuePropagation.cpp Show resolved Hide resolved
@jdmpapin jdmpapin added the depends:omr Pull request is dependent on a corresponding change in OMR label Nov 11, 2022
@jdmpapin
Copy link
Contributor

Please also handle call return type signatures in innerConstrainAcall()

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Nov 18, 2022

@jdmpapin @vijaysun-omr All comments are addressed. Ready for another review. Thanks!

@jdmpapin jdmpapin changed the title WIP: Add function to look for likely sub types WIP: Add functions to support and use type hint in VP Nov 24, 2022
@a7ehuo
Copy link
Contributor Author

a7ehuo commented Nov 29, 2022

@jdmpapin All comments are addressed. Ready for another review. Thanks!

jdmpapin
jdmpapin approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@jdmpapin jdmpapin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after one more small fix

runtime/compiler/optimizer/J9ValuePropagation.cpp Outdated Show resolved Hide resolved
@jdmpapin
Copy link
Contributor

@vijaysun-omr, this is ready for your review now

@a7ehuo @jdmpapin
Add functions to support and use type hint in VP
67eca14 
- Add `findLikelySubtype` to look for a likely sub type
  given a `TR_OpaqueClassBlock` pointer or a class signature.
- Add `createTypeHintConstraint` to create a constraint if
  a likely sub type for a given class signature is found
- Create type hint for parameters in `getParmValues`
- Create type hint using return type in `innerConstrainAcall`

Co-Authored-By: Devin Papineau <devin@ajdmp.ca>
Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
@vijaysun-omr
Copy link
Contributor

Jenkins test sanity all jdk8,jdk11,jdk19

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Dec 1, 2022

The build failed because it has dependency on eclipse-omr/omr#6801 which hasn't been promoted to openj9-omr yet

@vijaysun-omr
Copy link
Contributor

Yes I should have waited for that to happen.

@a7ehuo
Copy link
Contributor Author

a7ehuo commented Dec 2, 2022

eclipse-omr/omr#6801 is now promoted to openj9-omr

@jdmpapin
Copy link
Contributor

jdmpapin commented Dec 2, 2022

Jenkins test sanity all jdk8,jdk11,jdk19

@jdmpapin
Copy link
Contributor

jdmpapin commented Dec 2, 2022

I think you can probably remove WIP from the title now btw

@pshipton
Copy link
Member

pshipton commented Dec 2, 2022

eclipse-omr/omr#6801 is now promoted to openj9-omr

It's not promoted yet, but should be shortly.
https://openj9-jenkins.osuosl.org/job/Pipeline-OMR-Acceptance/428/

@jdmpapin
Copy link
Contributor

jdmpapin commented Dec 2, 2022

Oh... I followed the link, but didn't check that it lead to the openj9 branch 🤦

@pshipton
Copy link
Member

pshipton commented Dec 2, 2022

The OMR change is promoted now.

@a7ehuo a7ehuo changed the title WIP: Add functions to support and use type hint in VP Add functions to support and use type hint in VP Dec 2, 2022
@jdmpapin
Copy link
Contributor

jdmpapin commented Dec 2, 2022

Jenkins test sanity all jdk8,jdk11,jdk19

1 similar comment
@vijaysun-omr
Copy link
Contributor

Jenkins test sanity all jdk8,jdk11,jdk19

@vijaysun-omr vijaysun-omr merged commit 7c8cdf7 into eclipse-openj9:master Dec 5, 2022
@a7ehuo a7ehuo deleted the vpclass-hint-interface-1 branch February 7, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdmpapin jdmpapin jdmpapin approved these changes

@vijaysun-omr vijaysun-omr vijaysun-omr approved these changes

Assignees
No one assigned
Labels
comp:jit depends:omr Pull request is dependent on a corresponding change in OMR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@a7ehuo @jdmpapin @vijaysun-omr @pshipton