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

[Vladimir Ivanov]

Thanks, Vladimir!

Best regards,
Vladimir Ivanov

On 01/02/2019 18:27, Vladimir Kozlov wrote:
> 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