RFR: 8151661: Performance regression on Solaris-SPARC in 9-b103

Rahul Raghavan rahul.v.raghavan at oracle.com
Wed Jun 8 07:14:52 UTC 2016


<cc: aarch64-port-dev at openjdk.java.net>


Hi,

Please review following patch for JDK- 8151661.

<webrev.00> :  http://cr.openjdk.java.net/~rraghavan/8151661/webrev.00/


1. <bug details> : https://bugs.openjdk.java.net/browse/JDK-8151661
Performance regression reported with internal benchmark harness on Solaris-SPARC in JDK9-b103 build onwards.


2. Found that following adlc change done as part of old JDK-8145438 fix as the root cause of the Performance regression.
    (https://bugs.openjdk.java.net/browse/JDK-8145438 - Guarantee failures since 8144028: Use AArch64 bit-test instructions in C2)
    (https://bugs.openjdk.java.net/browse/JDK-8144028 - Use AArch64 bit-test instructions in C2)
    (http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/558ddc947c8d )
    [src/share/vm/adlc/formssel.cpp]
       bool InstructForm::check_branch_variant(ArchDesc &AD, InstructForm *short_branch) {
         if (_matrule != NULL &&
             this != short_branch && // Don't match myself
             !is_short_branch() && // Don't match another short branch variant
              reduce_result() != NULL &&
             strcmp(reduce_result(), short_branch->reduce_result()) == 0 &&
      - _matrule->equivalent(AD.globalNames(), short_branch->_matrule)) {
      + _matrule->equivalent(AD.globalNames(), short_branch->_matrule) &&
      + equivalent_predicates(this, short_branch)) {
           // The instructions are equivalent.


3. The extra 'equivalent_predicates()' check added is not working for following 'long / short' branchNode variants combinations for SPARC.
(in sparc.ad - only the short variants got 'predicate(UseCBCond)' and no predicate set for long variants)
            branchNode / branch_shortNode
            cmpI_reg_branchNode / cmpI_reg_branch_shortNode
            cmpI_imm_branchNode / cmpI_imm_branch_shortNode
            cmpU_reg_branchNode / cmpU_reg_branch_shortNode
            cmpU_imm_branchNode / cmpU_imm_branch_shortNode
            cmpL_reg_branchNode / cmpL_reg_branch_shortNode
            cmpL_imm_branchNode / cmpL_imm_branch_shortNode
            cmpP_reg_branchNode / cmpP_reg_branch_shortNode
            cmpP_null_branchNode / cmpP_null_branch_shortNode
            cmpN_reg_branchNode / cmpN_reg_branch_shortNode
            cmpN_null_branchNode / cmpN_null_branch_shortNode
            cmpI_reg_branchLoopEndNode / cmpI_reg_branchLoopEnd_shortNode
            cmpI_imm_branchLoopEndNode / cmpI_imm_branchLoopEnd_shortNode


4. http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-December/020450.html
>From this old 8145438 - RFR email thread, seems the adlc change was done independently, not as an integral part of original 8145438/8144028 -fix.
The same change was done based on the assumption that short and long branch variants cannot have different predicates.
But this is wrong for above SPARC cases.
Reverting only the adlc change, could not find any issues with -
    JPRT testing (default -testset hotspot) , 'hotspot/test/compiler/codegen/8144028/BitTests.java' jtreg test run for Sparc, AArch64)

So in general predicates for short and long branches could be different and should not be compared for check_branch_variant()!
The above proposed webrev.00 fix proposal is the same reverting earlier 8145438 adlc change.

Thanks,
Rahul


More information about the hotspot-compiler-dev mailing list