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

Roland Westrelin roland.westrelin at oracle.com
Wed Sep 2 08:56:16 UTC 2015


Thanks for looking at this, Vladimir.

> The fix seems correct. I think it req() was used assuming that new slices will be added only in following code. And then Note2: was added when I realized that previous code may add them but req() was not changed.
> 
> What is node 129? MergeMem and Phi 233 point to it.

It’s:

129    Proj    ===  126  [[ 276  235  142  143  233  234  275  274  175  188 ]] #2  Memory: @BotPTR *+bot, idx=Bot; !orig=[166],[273],[196] !jvms: TestEABadMergeMem::test @ bci:26
126    CallStaticJava  ===  116  43  57  8  1 ( 1  123  1  1  1  15  16  49  1  1 ) [[ 127  128  129 ]] # Static  TestEABadMergeMem::m_notinlined void (  ) TestEABadMergeMem::test @ bci:26 !jvms: TestEABadMergeMem::test @ bci:26

The first m_notinlined() call

> How node 129 references StoreI 110 and StoreI 125? StoreI 110 has 2 users, may be there is bypass.

The call’s memory is:

57     MergeMem        === _  1  7  47  125  [[ 126 ]]  { N47:rawptr:BotPTR N125:TestEABadMergeMem$Box+12 * }  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestEABadMergeMem::test @ bci:4

So 129 gets to 110 through 125.

The other use for 110 is:

113    MergeMem        === _  1  7  47  110  [[ 118 ]]  { N47:rawptr:BotPTR N110:TestEABadMergeMem$Box+12 * }  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestEABadMergeMem::test @ bci:23

which is used by an uncommon trap:

118    CallStaticJava  ===  117  43  113  8  9 ( 99  1  1  1  1  1  1  1  1  1  1  91  14 ) [[ 119 ]] # Static uncommon_trap(reason='null_check' action='maybe_recompile')  void ( int ) C=0.000100 TestEABadMergeMem::test @ bci:23 reexecute !jvms: TestEABadMergeMem::test @ bci:23

for the null check of c.i = k

Roland.

> 
> Thanks,
> Vladimir
> 
> On 8/31/15 8:33 AM, Roland Westrelin wrote:
>> 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.
>> 



More information about the hotspot-compiler-dev mailing list