Skip to content

Add type hint class to VP - #6801

Merged
vijaysun-omr merged 1 commit into
eclipse-omr:masterfrom
a7ehuo:vpclass-hint-interface-8
Dec 1, 2022
Merged

Add type hint class to VP#6801
vijaysun-omr merged 1 commit into
eclipse-omr:masterfrom
a7ehuo:vpclass-hint-interface-8

Conversation

@a7ehuo

@a7ehuo a7ehuo commented Nov 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Type hint class is added to VPClass and VPClassType. It suggests
the value is LIKELY an exact type. It could be used along with
runtime checks on this speculation for other optimizations.

This change also adds the following APIs:

  • 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

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

@a7ehuo
a7ehuo requested a review from vijaysun-omr as a code owner November 4, 2022 16:14
@a7ehuo a7ehuo mentioned this pull request Nov 4, 2022
Merged
1 task
@a7ehuo
a7ehuo requested a review from jdmpapin November 4, 2022 16:18
@a7ehuo

a7ehuo commented Nov 4, 2022 •
edited by jdmpapin
Loading

Copy link
Copy Markdown
Contributor Author

This PR is related to eclipse-openj9/openj9#16265 which implements findLikelySubtype and createTypeHintConstraint in OpenJ9, but not depend on eclipse-openj9/openj9#16265.

@vijaysun-omr @jdmpapin May I ask you to review this change? Thank you!
@hzongaro fyi

@a7ehuo

a7ehuo commented Nov 10, 2022

Copy link
Copy Markdown
Contributor Author

@vijaysun-omr @jdmpapin Just following up on the review of this PR. Thank you!

jdmpapin
jdmpapin reviewed Nov 11, 2022
View reviewed changes

@jdmpapin jdmpapin left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread compiler/optimizer/VPConstraint.cpp Outdated
Comment thread compiler/optimizer/VPConstraint.cpp
Comment thread compiler/optimizer/VPConstraint.cpp Outdated
Comment thread compiler/optimizer/VPConstraint.cpp Outdated
Comment thread compiler/optimizer/VPConstraint.cpp Outdated
Comment thread compiler/optimizer/VPConstraint.cpp
Comment thread compiler/optimizer/VPHandlers.cpp Outdated
Comment thread compiler/optimizer/VPHandlers.cpp Outdated
@a7ehuo
a7ehuo force-pushed the vpclass-hint-interface-8 branch from f2799c7 to bb6ab93 Compare November 18, 2022 16:35
@a7ehuo

a7ehuo commented Nov 18, 2022

Copy link
Copy Markdown
Contributor Author

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

jdmpapin
jdmpapin approved these changes Nov 24, 2022
View reviewed changes

@jdmpapin jdmpapin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, with two very minor (optional) comments

Comment thread compiler/optimizer/VPHandlers.cpp Outdated
Comment thread compiler/optimizer/VPHandlers.cpp Outdated
@jdmpapin

jdmpapin commented Nov 24, 2022

Copy link
Copy Markdown
Contributor

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

Annabelle Huo @jdmpapin Annabelle Huo
Add type hint class to VP
9b0638d 
Type hint class is added to `VPClass` and `VPClassType`.
It suggests the value is LIKELY an exact type. It could be
used along with runtime checks on this speculation for
other optimizations.

This change also adds the following APIs:
- 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

Co-Authored-By: Devin Papineau <devin@ajdmp.ca>
Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
@a7ehuo
a7ehuo force-pushed the vpclass-hint-interface-8 branch from bb6ab93 to 9b0638d Compare November 25, 2022 16:24
@vijaysun-omr

vijaysun-omr commented Nov 30, 2022

Copy link
Copy Markdown
Contributor

Jenkins build all

@a7ehuo

a7ehuo commented Dec 1, 2022

Copy link
Copy Markdown
Contributor Author

linux_ppc-64_le failed. It might be related to #6571

[2022-11-30T19:28:18.095Z] 31: free(): invalid next size (normal)
[2022-11-30T19:28:18.095Z] 31/31 Test #31: compunittest ......................Child aborted***Exception:   0.75 sec
[2022-11-30T19:28:18.095Z]

linux_riscv64 failed with the following error message. Looks the same issue as #6704

[2022-11-30T23:52:31.878Z] 21: /home/jenkins/workspace/Build/fvtest/threadtest/rwMutexTest.cpp:707: Failure
[2022-11-30T23:52:31.878Z] 21: Value of: 1 == infoReader1->readCounter
[2022-11-30T23:52:31.878Z] 21:   Actual: false
[2022-11-30T23:52:31.878Z] 21: Expected: true
[2022-11-30T23:52:31.878Z] 21: [  FAILED  ] RWMutex.AllReadersProceedTest (3016 ms)

@vijaysun-omr

vijaysun-omr commented Dec 1, 2022

Copy link
Copy Markdown
Contributor

Thanks Annabelle. I am merging since these are known problems, other checks have passed, and reviews are also done.

@vijaysun-omr
vijaysun-omr merged commit 6810631 into eclipse-omr:master Dec 1, 2022
@a7ehuo
a7ehuo deleted the vpclass-hint-interface-8 branch June 6, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vijaysun-omr vijaysun-omr vijaysun-omr approved these changes
+1 more reviewer
@jdmpapin jdmpapin jdmpapin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp:compiler

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@a7ehuo @jdmpapin @vijaysun-omr