-
Notifications
You must be signed in to change notification settings - Fork 720
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
Remove Q type checks in Valhalla code #18157
Comments
When "primitive" is removed, Since only null-restricted field can be potentially flattened, we can add a check of null restricted field ( |
I am not sure right away, but I can check what is used by GC for recognition of VT "leaf object" (does not introduce new object references). We are going to need the way to make "leaf object" detection for VT objects in order to have GC optimization running. |
Not sure about the "we can use J9ROMCLASS_IS_VALUE() instead" part. There is a |
Before removing Q type checks I'm modifying existing Valhalla functional tests to be compatible as much as possible with the lw5 compiler. edit: tests in src_lw5 compile with the latest version of lw5. Additional vm support is needed for them to pass. No changes are needed for these files, they will remain in src:
These tests have been split into src_qtypes and src_lw5 and compile with lw5 commit dc715159feda32945d5c2351ede9c6be04a64d6f:
|
Q type is going to be removed. Instead we should do null restricted modifier check. In order for existing test to work, this change does not remove q type checks from the existing code. NULL restricted check is appended to places where Q type check is done. issue eclipse-openj9#18157 Signed-off-by: Hang Shao <hangshao@ca.ibm.com>
Edit: given that q is removed in the current javac and null restricted support is not yet there tests in src_qtypes have been updated and no longer have dependencies on q. This pr can be closed once all references to q and primitive are removed. |
Related to: eclipse-openj9/openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Opened eclipse-omr/omr#7399 |
The following q and primitive reference are still outstanding before this issue can be closed: ReferenceType naming
J9_IS_J9CLASS_PRIMITIVE_VALUETYPE
J9::ClassEnv::isPrimitiveValueTypeClass #20398 TR_J9VMBase::checkArrayCompClassPrimitiveValueType
J9ClassIsPrimitiveValueType flag
fyi @a7ehuo I'm sure you're already aware of these to be removed when it makes sense to do so, just wanted to summarize the remaining work that I'm aware of. |
Also updated code comments where the old primitive value type is referenced Related: eclipse-openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Being removed in
Removed in |
Also updated code comments where the old primitive value type is referenced Related: eclipse-openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Also updated code comments where the old primitive value type is referenced Related: eclipse-openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Also updated code comments where the old primitive value type is referenced Related: eclipse-openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Also updated code comments where the old primitive value type is referenced Related: eclipse-openj9#18157 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
Currently many places in the VM rely on Q type check for the null-restricted value type. The Q type is going to be removed in the future. L type will be used for all value types, no matter it is null-restricted or not. For the fields, the Q type is going to be replaced with null-restricted attribute,
Also the keyword "primitive" is going to be removed.
The null restricted fields will be treated the same as Q type fields. For now, we need to add null-restricted attribute checks into the places where we are doing Q type field checks. After javac is updated, we can remove the Q type checks completely.
Some of our testing code is generating Q types and using "primitive", they need to be updated as well.
The text was updated successfully, but these errors were encountered: