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

[Vladimir Kozlov]

On 2/1/19 9:41 AM, Vladimir Ivanov wrote:
> Thanks, Vladimir.
> 
>> Nice! Did you consider to update indexes in separate pass at the end of PhaseRenumberLive? Would it be simpler?
> 
> It already does additional pass, but only over the nodes which contain embedded IDs (those are recorded into _delayed 
> during the first pass). If you are talking about full pass over the graph, then it would allow to eliminate _delayed array.

No, I was only concern about code in new_index() where you store _live_node_count if Id is not set in _old2new_map.
I was thinking to collect all nodes with ID (regardless values in _old2new_map) first as you do now for _delayed. And 
then do the same as your code (call update_embedded_ids() in post loop) where you can assume that _old2new_map contains 
all values - so you can replace your checks with asserts. Am I missing something?

Vladimir K

> 
> Also, it's possible to do everything in a single pass, but it requires eager allocation of IDs for not-yet-seen nodes 
> (which needs _new2old_map).
> 
> Best regards,
> Vladimir Ivanov
> 
>> On 1/31/19 11:10 AM, Vladimir Ivanov wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8217918
>>> http://cr.openjdk.java.net/~vlivanov/8217918/webrev.00/
>>>
>>> When -XX:+AggressiveUnboxing is enabled, LoadNode::split_through_phi() produces Phi nodes with non-negative 
>>> _inst_mem_id & _inst_id early enough, so it breaks PhaseRenumber pass which doesn't support nodes with embedded IDs.
>>>
>>> Proposed fix tracks nodes with embedded IDs and updates them once renumbering pass over the graph is over.
>>>
>>> Testing: hs-precheckin-comp, tier1-5
>>>
>>> Best regards,
>>> Vladimir Ivanov