RFR(S): 8211232: GraphKit::make_runtime_call() sometimes attaches wrong memory state to call

[Vladimir Kozlov]

Hi Roland,

I understand that you want to avoid second reset_memory() and I agree.
But I concern about your code for setting input memory for call. Why not to pass narrow_mem from 
memory(adr_type) to set_predefined_input_for_runtime_call() in this case and NULL in others and 
check for NULL to select which memory to set.  memory(adr_type) will check for merge_mem.

Thanks,
Vladimir

On 9/27/18 8:52 AM, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/8211232/webrev.00/
> 
> This came up in shenandoah testing with XX:+ExtendedDTraceProbes.
> 
> make_runtime_call() is called through make_dtrace_method_exit() from
> Parse::return_current(). Memory state at this point is:
> 
>   137 Phi === 135 _ _ 91 [[ 74 141 145 150 152 162 166 168 179 182 187 193 202 211 216 225 228 237 242 258 266 274 282 ]] #memory Memory: @BotPTR *+bot, idx=Bot; !jvms: IdentityHashMap::put @ bci:24
>   141 MergeMem === _ 1 137 1 1 279 1 275 282 [[ 142 ]] { - - N279:java/lang/Object+-8 * - N275:narrowoop: java/lang/Object *[int:>=0]+-8 * N282:narrowoop: java/lang/Object *[int:>=0]+any * [narrow] } Memory: @BotPTR *+bot, idx=Bot; !jvms: IdentityHashMap::put @ bci:24
> 
> The Phi is a loop phi so not all its inputs are set yet. The following
> code in GraphKit::make_runtime_call():
> 
>      assert(!wide_out, "narrow in => narrow out");
>      Node* narrow_mem = memory(adr_type);
>      prev_mem = reset_memory();
>      map()->set_memory(narrow_mem);
> 
> set the entire memory state to the phi. Next in
> GraphKit::set_predefined_input_for_runtime_call():
> 
>    Node* memory = reset_memory();
> 
> causes the current memory state (the Phi) to be transformed which the
> GVN transforms to:
> 
>   91 Phi === 89 _ _ 73 [[ 137 100 103 105 113 116 118 126 129 131 ]] #memory Memory: @BotPTR *+bot, idx=Bot; !jvms: IdentityHashMap::put @ bci:24
> 
> the out of loop memory state and so the wrong state.
> 
> Roland.
>