Make static final field folding more aggressive #19699
Conversation
jdmpapin
added
comp:jit
depends:omr
comp:jitserver
|
@vijaysun-omr, would you mind reviewing? |
|
@mpirvu since this includes some JIT server relevant changes |
|
Jenkins test sanity.functional all jdk11,jdk17 |
|
Builds failed because eclipse/omr#7376 hadn't (and still hasn't) promoted |
This can be used to prevent constant folding a specified set of static
final fields for debugging purposes or as a workaround.
Whenever the JIT attempts to fold a static final field, if this option
is set, it checks whether the glob pattern ("simple regex") matches
<class>.<field>:<sig>, e.g.
java/util/AraryList.EMPTY_ELEMENTDATA:[Ljava/lang/Object;
If it matches, then the static final field is not folded.
Starting in Java 17, it is no longer possible to remove the final attribute of a field to make it writable via reflection, and in classes with version 53 and later, putstatic can write to static final fields only during class initialization. With this justification, the JIT compiler has been treating static final fields with declared type VarHandle as reliable for folding (if declared in a suitable class). But the justification applies regardless of the declared type of the static final field, so with this commit, so too will the folding.
Whenever the compiler attempts to fold a static final field (that hasn't been explicitly allowlisted somehow), it will now determine whether the declaring class contains a putstatic instruction that could modify one of its static final fields outside of <clinit>. The presence of such a putstatic instruction prevents folding, but it should be extremely rare, since javac doesn't generate any. We can assume JCL classes won't attempt to store to static final fields outside of <clinit>, and classes with class file version 53 or later won't succeed even if they do attempt it. Other classes, i.e. non-JCL classes with version 52 or earlier, will have their bytecode scanned. If no putstatic instruction interferes, all primitive-typed static final fields will now be folded without any runtime assumption. If the field could potentially be modified later, e.g. using some reflection hack or Unsafe, the compiled code is not required to see the new value. This is also true of references, but due to limitations in the JIT's handling of known objects, those still require runtime assumptions if there is any possibility they will be modified later. Later on, when it becomes possible to treat reference-typed static final fields the same way, it should be possible to entirely eliminate guarded static final field folding.
jdmpapin
force-pushed
the
aggressive-sfff
branch
from
c4b3627
to
070557c
Compare
|
Updated to check for |
|
@mpirvu are you good with the changes Devin made ? If so, I will run tests again. |
|
Jenkins test sanity.functional all jdk11,jdk17 |
|
It looks like eclipse/omr#7376 still hadn't promoted when the PR builds were last restarted. But it has promoted by now, so I'll restart them again Jenkins test sanity.functional all jdk11,jdk17 |
|
Still some checks in progress, but the finished ones have all succeeded. Adding the same tests but with JITServer Jenkins test sanity plinuxjit,xlinuxjit jdk11,jdk17 |
|
Jenkins test sanity plinuxjit jdk11,jdk17 |
|
Jenkins test sanity xlinuxjit jdk17 |
|
Both of the aborted PPCLE builds just failed to run: In the console output, Jenkins test sanity plinuxjit jdk11,jdk17 |
|
Same problem. I see now that there's an ongoing problem with the PPC64LE server, and Linux PPC64LE seems to have been excluded from "all" - here the only builds on that platform are the ones that were explicitly launched for plinuxjit At this point we could run zlinuxjit, or we could say that xlinuxjit was enough testing for JITServer |
|
I think that xlinuxjit is sufficient |
|
Merging based on the comments above. Reviews were done some days back. |
|
There are no available plinux machines to build on, and no outlook for getting any. You'll need to run an internal build for this platform. |
dontFoldStaticFinalFields={}JIT optionVarHandleto all typesThis depends on eclipse/omr#7376