Enable peeking ILGen for param loads using NeedsPeekingHeuristic #21913
Conversation
|
@vijaysun-omr requesting review. |
|
Just started the review and I may well find the answer in the code, but could you please elaborate on " This commit also Why/is this a given ? |
|
Okay I understood from looking at the code. The setting of "needs peeking" to true just means that we don't need further analysis since no one will change it back to false once it is set to true. |
|
Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk21 |
|
@mpirvu this change has the potential to increase compile time (and throughput) because it increases the probability of needing to generate IL for peeking. |
|
There are two separate problems the PR builds revealed. The first problem is due to an issue with matching call sites with the correct call nodes. I have a working fix for that. The second problem is the change in NPH surfacing an issue with validating prex args we only run into due to the different inlining outcome resulting from the NPH change (in JDK8 only, as far as I can tell). We have seen such problems surfacing due to changes in inlining outcome in the past: #21465. I am currently working on fixing that issue. |
NPH keeps track of loads of parameters when the caller method has prex arg info, which is then used when we come across one of the invoke* bytecodes. If the param load is used as arg for the call, then we would want to propagate the prex arg info down the callee, for which peeking ILGen is necessary. This commit also terminates NPH once _needsPeeking is set as true, as any further analysis would not change the peeking outcome. Peeking is disabled by default for MethodHandle thunk archetype methods in JDK8 and JDK11, as they are not typical Java methods and require special handling in existing optimizations to refine and inline. Peeking ILGen due to param loads can be disabled using the new env option TR_disableNPH. Signed-off-by: Nazim Bhuiyan <[email protected]>
This commit disables checking for class compatibility for classes of LambdaForm generated methods during arg propagation, as the class lookup for such runtime generated classes would fail. For LF methods, name and signature matching is sufficient. Signed-off-by: Nazim Bhuiyan <[email protected]>
|
@vijaysun-omr I have fixed the 2 different issues observed in the last set of PR builds. The second problem turned out to be due to peeking ILGen for MH thunks that are part of the old MH implementation. Disabling peeking IL generation of MH thunk archetype methods fixes that problem. Requesting another review. |
|
Jenkins test sanity.functional,sanity.openjdk all jdk8,jdk21 |
|
Jenkins test sanity.functional zlinux jdk8 |
|
S390x Linux sanity.functional test failure: It is a known issue: #14706 |
|
Checks have all passed, after the restart. Merging. |
NPH keeps track of loads of parameters when the caller method has prex arg info, which is then used when we come across one of the invoke* bytecodes. If the param load is used as arg for the call, then we would want to propagate the prex arg info down the callee, for which peeking ILGen is necessary. This commit also terminates NPH once _needsPeeking is set as true, as any further analysis would not change the peeking outcome.