Inline Class.getComponentType and refactor IL generation to access classDepthAndFlags field of j9Class #19558
Conversation
Several methods in the JIT compiler use the name of the classDepthAndFlags field of J9Class in their names, but they incorrectly refer to it as classAndDepthFlags. This commit corrects the typographical errors in the naming of those methods, but keeps one instance that is used upstream in J9_PROJECT_SPECIFIC code in OMR until that can be removed.
Various places in the compiler need to generate IL to load the classDepthAndFlags field of j9class. Those locations need to take into account whether the target platform is 32-bit or 64-bit, as it affects the width of the field. Introducing methods in TR_J9VMBase to load the field and potentially test the settings of bits in it hides some of that.
|
@0xdaryl and @vijaysun-omr, may I ask you to review this change? This pull request should be considered in conjunction with the upstream draft pull request eclipse/omr#7334, as that pull request uses the new One thing I want to point out is that The question is whether it's worthwhile continuing to use the result in all three of those ways, or whether all the places that use the result of that IL should just test whether the result of masking is equal to zero, or just test whether the result is equal to the mask. If we preserve all three, it would probably make sense to hide those possibilities inside |
|
If the assert discussed is added, I will run testing and approve. Thanks |
I added that assertion in commit df1f93b. If you are OK with the change, I will squash that commit down.
Regarding this point, I discussed with both @0xdaryl and @vijaysun-omr off-line, and there was consensus that producing a zero/non-zero result and comparing that with zero make the most sense and likely yields the best performance. I will make that change in a follow-on pull request (possibly optionally allowing the result to be shifted to yield a result of zero or one.) |
|
Thanks, assert looks good to me. If you squash, I will run testing and consider merge. |
Check for the opportunity to inline calls to Class.getComponentType, using the underlying j9ArrayClass. In particular, if the Class is known to be an array class, the test for the J9AccClassArray flag in the classDepthAndFlags field can be skipped. Also, mark Object.getClass() as non-null during IL Generation. That prevents a NULLCHK from being being generated for the Class reference in an expression like o.getClass().getComponentType(). The presence of the NULLCHK would inhibit this transformation.
hzongaro
force-pushed
the
inline-getComponentType-access-classDepthAndFlags
branch
from
df1f93b
to
c34c2b5
Compare
Squashed. Thanks! |
|
Jenkins test sanity.functional all jdk8,jdk21 |
|
JDK 21 aarch64 mac build failed with the following error. This has been seen in builds before, and is unrelated to this change. Restarting. JDK 8 x86-64 Linux sanity.functional failure appears to be infrastructure-related. Restarting. |
|
Jenkins test sanity.functional jdk21 amac |
|
Jenkins test sanity.functional jdk8 xlinux |
|
Sigh! Swapped versions and platforms. Jenkins test sanity.functional amac jdk21 |
|
Jenkins test sanity.functional xlinux jdk8 |
|
It looks like all recent pull request testing for x86-64 Linux JDK 8 are hitting similar infrastructure issues as were encountered for this pull request. |
|
Given that this is a cross-platform fix, and all JDK21 testing has passed, I am going to merge this. |
|
xlinux should be fixed now. |
This change generates inline IL in Value Propagation for references to
Class.getComponentType(). That method was previously only inlined if the Class was known at compile-time. If the Class is known to be an array class at compile-time, the generated IL will load the<componentClass>from thej9ArrayClass; otherwise, it will generate a run-time test of whether the Class is an array class before returning the<componentClass>or a null reference.Calls to
Object.getClass()are also marked as non-null during IL generation as that method is known to always return a non-null reference. That exposes opportunities to optimize uses ofClassobjects that require the reference to be known to be non-null, including inlining calls toClass.getComponentType()in Value Propagation.Also, there are several places in the JIT compiler that generate IL to access the
classDepthAndFlagsfield ofj9class. This introduces methods inTR_J9VMBaseto do that, as has previously been done with some methods that generate IL to access theclassFlagsfield.Finally, this change also deals with some typos in methods that refer to
classDepthAndFlagsincorrectly asclassAndDepthFlags. One instance,J9::SymbolReferenceTable::findOrCreateClassAndDepthFlagsSymbolRef, will remain until its upstream use inJ9_PROJECT_SPECIFICcode in OMR is removed in pull request eclipse/omr#7334.