[13] RFR (S): 8217918: C2: -XX:+AggressiveUnboxing is broken

[Vladimir Kozlov]

Okay. Got it. You answered all my concerns ;)
Reviewed.

Thanks,
Vladimir

On 2/1/19 6:12 PM, Vladimir Ivanov wrote:
> 
>>>> This ID value can't be random one.
>>>
>>> The code you refer to compares PHI nodes using is_same_inst_field() which looks for the node with the same inst_id 
>>> (among other things).
>>>
>>> Placeholder ID is chosen to be unique (no node in the graph with the same ID is present or will be in the future), 
>>> but the ID is shared between all the embedded usages. It doesn't refer to any existing node, but it didn't before the 
>>> renumbering pass as well.
>>
>> But before renumbering it could be the same ID - and now it will be different. What I am trying to say some nodes 
>> could have the same ID which came from already dead node. I think we should preserve that - keep the same ID (which 
>> could be different after renumbering.
> 
> Yes, that's exactly how it works after the fix:
> 
> +int PhaseRenumberLive::new_index(int old_idx) {
> +  assert(_is_pass_finished, "not finished");
> +  if (_old2new_map.at(old_idx) == -1) { // absent
> +    // Allocate a placeholder to preserve uniqueness
> +    _old2new_map.at_put(old_idx, _live_node_count);
> +    _live_node_count++;
> +  }
> +  return _old2new_map.at(old_idx);
> +}
> 
> PhaseRenumberLive::new_index() consults _old2new_map first and updates it with placeholder ID. All consequent requests 
> will see it.
> 
> Best regards,
> Vladimir Ivanov