Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8243670

Unexpected test result caused by C2 MergeMemNode::Ideal

    Details

    • Subcomponent:
    • Resolved In Build:
      b31
    • CPU:
      generic
    • OS:
      generic
    • Verification:
      Verified

      Backports

        Description

        Witnessed different results for TestReplaceEquivPhis under three different execution mode.

        Case1> $ java -Xint TestReplaceEquivPhis
        v = 2
        v = 2
        v = 2
        v = 2
        v = 2

        Case2> $ java -Xcomp -XX:CompileOnly=TestReplaceEquivPhis.test -XX:-BackgroundCompilation TestReplaceEquivPhis
        v = 1
        v = 2
        v = 2
        v = 2
        v = 2

        Case3> $ java -Xcomp -XX:CompileOnly=TestReplaceEquivPhis.test -XX:-BackgroundCompilation -XX:-SplitIfBlocks TestReplaceEquivPhis
        v = 1
        v = 1
        v = 1
        v = 1
        v = 1

        For case2, C2 generated incorrect code for the following method:
            420 4 % b 4 TestReplaceEquivPhis::test @ 25 (107 bytes)

         v = iFld; // load from field "iFld"
         iFld = TestReplaceEquivPhis.instanceCount; // store to field "iFld"

        JIT assembly code:
        3319 152 B20: # out( B21 ) <- in( B19 ) Freq: 8.80219
        3320 152 addl RBX, #2 # int
        3321 155 xorl R10, R10 # int
        3322 158 xorl R11, R11 # int
        3323 15b cmpl RBX, R10
        3324 15e cmovllt RBX, R11 # max
        3325 162 movl R10, #177 # int
        3326 168 cmpl RBX, R10
        3327 16b movl R11, #177 # int
        3328 171 cmovlgt RBX, R11 # min
        3329
        3330 175 B21: # out( B55 B22 ) <- in( B20 B26 ) Loop( B21-B26 inner pre of N678) Freq: 17.6043
        3331 175 movl R10, [RDI + #120 (8-bit)] # int ! Field: volatile Test.instanceCount
        3332 179 MEMBAR-acquire ! (empty encoding)
        3333 179 movl [R8 + #12 (8-bit)], R10 # int ! Field: Test.iFld
        3334 17d movl R9, [RDI + #112 (8-bit)] # compressed ptr ! Field: Test.iArrFld
        3335 181 movl R11, [R12 + R9 << 3 + #12] (compressed oop addressing) # range
        3336 186 NullCheck R9
        3337
        3338 186 B22: # out( B46 B23 ) <- in( B21 ) Freq: 17.6043
        3339 186 movl RBP, [R8 + #12 (8-bit)] # int ! Field: Test.iFld
        3340 18a movl [rsp + #0], RBP # spill
        3341 18d decode_heap_oop_not_null R9,R9
        3342 191 cmpl R14, R11 # unsigned
        3343 194 jnb,u B46 P=0.000001 C=-1.000000
        3344
        3345 19a B23: # out( B56 B24 ) <- in( B22 ) Freq: 17.6043
        3346 19a movl [R9 + #16 + R14 << #2], #0 # int
        3347 1a3 movl R11, [RDI + #120 (8-bit)] # int ! Field: volatile Test.instanceCount
        3348 1a7 MEMBAR-acquire ! (empty encoding)
        3349 1a7 addl R11, R10 # int
        3350 1aa movl [R8 + #12 (8-bit)], R11 # int ! Field: Test.iFld
        3351 1ae movl R11, [RDI + #112 (8-bit)] # compressed ptr ! Field: Test.iArrFld
        3352 1b2 movl R10, [R12 + R11 << 3 + #12] (compressed oop addressing) # range
        3353 1b7 NullCheck R11

        Proposed C2 fix which is under testing:
        diff -r c2bfa816365f src/hotspot/share/opto/memnode.cpp
        --- a/src/hotspot/share/opto/memnode.cpp Sat Apr 25 18:11:08 2020 -0700
        +++ b/src/hotspot/share/opto/memnode.cpp Mon Apr 27 22:34:25 2020 +0800
        @@ -4605,24 +4605,6 @@
             }
             // else preceding memory was not a MergeMem

        - // replace equivalent phis (unfortunately, they do not GVN together)
        - if (new_mem != NULL && new_mem != new_base &&
        - new_mem->req() == phi_len && new_mem->in(0) == phi_reg) {
        - if (new_mem->is_Phi()) {
        - PhiNode* phi_mem = new_mem->as_Phi();
        - for (uint i = 1; i < phi_len; i++) {
        - if (phi_base->in(i) != phi_mem->in(i)) {
        - phi_mem = NULL;
        - break;
        - }
        - }
        - if (phi_mem != NULL) {
        - // equivalent phi nodes; revert to the def
        - new_mem = new_base;
        - }
        - }
        - }
        -
             // maybe store down a new value
             Node* new_in = new_mem;
             if (new_in == new_base) new_in = empty_mem;

        JIT assembly with the patch:
        3317 152 B20: # out( B21 ) <- in( B19 ) Freq: 8.80219
        3318 152 addl RBX, #2 # int
        3319 155 xorl R10, R10 # int
        3320 158 xorl R11, R11 # int
        3321 15b cmpl RBX, R10
        3322 15e cmovllt RBX, R11 # max
        3323 162 movl R11, #177 # int
        3324 168 cmpl RBX, R11
        3325 16b movl R10, #177 # int
        3326 171 cmovlgt RBX, R10 # min
        3327
        3328 175 B21: # out( B55 B22 ) <- in( B20 B26 ) Loop( B21-B26 inner pre of N678) Freq: 17.6043
        3329 175 movl R10, [RDI + #120 (8-bit)] # int ! Field: volatile Test.instanceCount
        3330 179 movl R11, [R8 + #12 (8-bit)] # int ! Field: Test.iFld
        3331 17d movl [rsp + #0], R11 # spill
        3332 181 MEMBAR-acquire ! (empty encoding)
        3333 181 movl [R8 + #12 (8-bit)], R10 # int ! Field: Test.iFld
        3334 185 movl RSI, [RDI + #112 (8-bit)] # compressed ptr ! Field: Test.iArrFld
        3335 188 movl R11, [R12 + RSI << 3 + #12] (compressed oop addressing) # range
        3336 18d NullCheck RSI
        3337
        3338 18d B22: # out( B46 B23 ) <- in( B21 ) Freq: 17.6043
        3339 18d decode_heap_oop_not_null RCX,RSI
        3340 191 cmpl R14, R11 # unsigned
        3341 194 jnb,u B46 P=0.000001 C=-1.000000
        3342
        3343 19a B23: # out( B56 B24 ) <- in( B22 ) Freq: 17.6043
        3344 19a movl [RCX + #16 + R14 << #2], #0 # int
        3345 1a3 movl R11, [RDI + #120 (8-bit)] # int ! Field: volatile Test.instanceCount
        3346 1a7 MEMBAR-acquire ! (empty encoding)
        3347 1a7 addl R11, R10 # int
        3348 1aa movl [R8 + #12 (8-bit)], R11 # int ! Field: Test.iFld
        3349 1ae movl R11, [RDI + #112 (8-bit)] # compressed ptr ! Field: Test.iArrFld
        3350 1b2 movl R10, [R12 + R11 << 3 + #12] (compressed oop addressing) # range
        3351 1b7 NullCheck R11


        public class TestReplaceEquivPhis {

            public static final int N = 400;
            public static volatile int instanceCount = 0;
            public int iFld = 0;
            public static int iArrFld[] = new int[N];

            public int test() {
                int v = 0;
                boolean bArr[] = new boolean[N];

                for (int i = 1; i < 344; i++) {
                    iFld = i;
                    for (int j = 2; j <177 ; j++) {
                        v = iFld;
                        iFld = TestReplaceEquivPhis.instanceCount;
                        TestReplaceEquivPhis.iArrFld[i] = 0;
                        iFld += TestReplaceEquivPhis.instanceCount;
                        TestReplaceEquivPhis.iArrFld[i] = 0;
                        bArr[j] = false;
                        TestReplaceEquivPhis.instanceCount = 1;

                        for (int k = 1; k < 3; k++) {
                            // do nothing
                        }
                    }
                }
                return v;
            }

            public static void main(String[] args) {
                    TestReplaceEquivPhis obj = new TestReplaceEquivPhis();
                    for (int i = 0; i < 5; i++) {
                        int result = obj.test();
                        System.out.println("v = " + result);
                        //if (result != 2) {
                        // throw new RuntimeException("Test failed.");
                        //}
                    }
                    //System.out.println("Test passed.");
            }

        }

          Attachments

            Issue Links

              Activity

                People

                • Assignee:
                  fyang Fei Yang
                  Reporter:
                  fyang Fei Yang
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  6 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: