JDK-8243670 : Unexpected test result caused by C2 MergeMemNode::Ideal
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: openjdk8u262,11,15
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2020-04-27
  • Updated: 2025-01-13
  • Resolved: 2020-07-04
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 15 JDK 16 JDK 8
11.0.11-oracleFixed 15 b31Fixed 16Fixed 8u371Fixed
Related Reports
Duplicate :  
Duplicate :  
Relates :  
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.");
    }

}
Comments
Fix request (11u) I would like to downport this for parity with 11.0.11-oracle. Prerequisite is the downport of JDK-8240795 Applies clean. I need to be sponsored please.
22-12-2020

URL: https://hg.openjdk.java.net/jdk/jdk15/rev/b54edbc03f86 User: fyang Date: 2020-07-04 03:21:26 +0000
04-07-2020

~Rahul Raghavan: Yes, I think so.
19-05-2020

Awaiting reviews? - http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-April/038008.html http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-May/038152.html
19-05-2020

Proposed fix pushed to submit repo and test result received looks good: Job: mach5-one-fyang-JDK-8243670-20200512-2345-10990238 BuildId: 2020-05-12-2344071.felix.yang.source No failed tests Tasks Summary • EXECUTED_WITH_FAILURE: 0 • NOTHING_TO_RUN: 0 • KILLED: 0 • HARNESS_ERROR: 0 • FAILED: 0 • PASSED: 101 • UNABLE_TO_RUN: 0 • NA: 0
13-05-2020

ILW = Incorrect result of C2 compiled code, edge case but easy to reproduce, no workaround but disable compilation with C2 = HMM = P2
04-05-2020

Here for this testcase, anti-dependence between load and store of field "iFld" is missing with the replace equivalent phis transformation in MergeMemNode::Ideal. With this transformation, PhaseCFG::insert_anti_dependences won't insert an anti-dependence edge between load and store in order to place the load correctly. v = iFld; // load from field "iFld" iFld = TestReplaceEquivPhis.instanceCount; // store to field "iFld"
27-04-2020