RFR(S): 8134031: Incorrect JIT compilation of complex code with inlining and escape analysis

[Roland Westrelin]

That bug is caused by a bad rewiring of memory edges during ConnectionGraph::split_unique_types()

http://cr.openjdk.java.net/~roland/8134031/webrev.00/

See test case. Before EA, the “test” method has 3 stores to 3 different Box Objects:

108    StoreI  ===  97  47  107  12  [[ 110 ]]  @TestEABadMergeMem$Box+12 *, name=i, idx=4;  Memory: @TestEABadMergeMem$Box:NotNull+12 *, name=i, idx=4; !jvms: TestEABadMergeMem::test @ bci:11
110    StoreI  ===  97  108  109  13  [[ 125  113 ]]  @TestEABadMergeMem$Box+12 *, name=i, idx=4;  Memory: @TestEABadMergeMem$Box:NotNull:exact+12 *, name=i, idx=4; !jvms: TestEABadMergeMem::test @ bci:17
125    StoreI  ===  116  110  124  14  [[ 57 ]]  @TestEABadMergeMem$Box+12 *, name=i, idx=4;  Memory: @TestEABadMergeMem$Box:NotNull+12 *, name=i, idx=4; !jvms: TestEABadMergeMem::test @ bci:23

chained through their memory inputs.

It also has 2 loads from 2 different Box objects:

248    LoadI   === _  233  109  [[ 249 ]]  @TestEABadMergeMem$Box+12 *, name=i, idx=4; #int !jvms: TestEABadMergeMem::test @ bci:87
246    LoadI   === _  233  124  [[ 249 ]]  @TestEABadMergeMem$Box+12 *, name=i, idx=4; #int !jvms: TestEABadMergeMem::test @ bci:82

The memory input of both loads is a Phi:

233    Phi     ===  153  274  129  [[ 150  21  246  248 ]]  #memory  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestEABadMergeMem::test @ bci:76

Input 1 of the Phi is:

274    MergeMem        === _  1  129  1  1  1  1  275  276  [[ 233 ]]  { - - - - N275:java/lang/Class:exact+104 * N276:java/lang/Class:exact+108 * }  Memory: @BotPTR *+bot, idx=Bot; !orig=[196] !jvms: TestEABadMergeMem::test @ bci:47

The Box that is allocated in the “test” method doesn’t escape. ConnectionGraph::split_unique_types() assigns a unique type to that allocation and rewires the memory edges.

In:

  //  Phase 2:  Process MemNode's from memnode_worklist. compute new address type and
  //            compute new values for Memory inputs  (the Memory inputs are not
  //            actually updated until phase 4.)

LoadI 248 is processed with: 

Node *mem = find_inst_mem(n->in(MemNode::Memory), alias_idx, orig_phis);

which goes through the Phi 233 to MergeMem 274 and in ConnectionGraph::find_inst_mem(), an edge is added to the MergeMem node:

        result = find_inst_mem(result, alias_idx, orig_phis);
        mmem->set_memory_at(alias_idx, result);

274 MergeMem        === _  1  129  1  1  1  1  275  276  110  [[ 233 ]]  { - - - - N275:java/lang/Class:exact+104 * N276:java/lang/Class:exact+108 * N110:TestEABadMergeMem$Box:NotNull:exact+12 *,iid=32 }  Memory: @BotPTR *+bot, idx=Bot; !orig=[196] !jvms: TestEABadMer\
geMem::test @ bci:47

for the new unique type.

In:

  //  Phase 3:  Process MergeMem nodes from mergemem_worklist.
  //            Walk each memory slice moving the first node encountered of each
  //            instance type to the the input corresponding to its alias index.

MergeMem 274 is processed. We go over all of its inputs, including the newly added one above. When that input is processed, the:

     while (mem->is_Mem()) {

loop iterates until store 108 (110 is for the new unique type) and the MergeMem is updated again:

MergeMem        === _  1  129  1  108  1  1  275  276  110  [[ 233 ]]  { - N108:TestEABadMergeMem$Box+12 * - - N275:java/lang/Class:exact+104 * N276:java/lang/Class:exact+108 * N110:TestEABadMergeMem$Box:NotNull:exact+12 *,iid=32 }  Memory: @BotPTR *+bot, idx=Bot;\
 !orig=[196] !jvms: TestEABadMergeMem::test @ bci:47

But now the MergeMem is incorrect: LoadI 246 now takes its memory state from that MergeMem through Phi 233 and for LoadI 246, the memory state is StoreI 108 which is before StoreI  125 that sets the field.

The compiler then uses split through Phi to optimize the if (flag3) { test in the test case which causes LoadI 246 to move through Phi 233 and a bad value to be loaded.

I think the root cause is that we process the input added to the MergeMem in Phase 2 in Phase 3. The fix I propose prevents that.

Roland.