RFC: C2: Anti-dependence on a load with a control in presence of a membar

[Vladimir Kozlov]

On 3/6/18 11:21 AM, Vladimir Kozlov wrote:
> This changes everything. Load is associated with non-global-escaping 
> allocation #311 (iid is assigned only in such cases). It is allowed its 
> memory edge change in such way.
> 
> Why GCM makes unschedulable graph? I don't see a problem in 
> 05_after_matching.png.

Is it because Load's memory (#173) is above membar (#317) but the Load 
below because of control?

Vladimir K

> 
> Vladimir K
> 
> On 3/6/18 10:51 AM, Vladimir Ivanov wrote:
>>
>>> There were several bugs before when we had trouble with loads which 
>>> have control edge. As I remember we only require RAW loads to have 
>>> such edges. Meaning Load nodes should have only dependency on memory 
>>> state. Of cause, there could be exclusions.
>>>
>>> Originally EA can skip all membars for instance's load because it 
>>> assumes that it will end-up in Store node into allocated object which 
>>> should *follow* instance's allocation. And it can skip membars (which 
>>> follow allocation) because nobody see non-escaping allocation.
>>>
>>> Load (#391) is not instance load from instance array (#363). It is 
>>> load from source Arraycopy (#255) (it is not allocation). So it 
>>> should not have bypass membars separating them:
>>>
>>> http://hg.openjdk.java.net/jdk/hs/file/4e82736053ae/src/hotspot/share/opto/escape.cpp#l2698 
>>
>>
>>
>> Updated IR dump during before/after split_unique_types with wider 
>> context (and, unfortunately, different node ids):
>>
>> http://cr.openjdk.java.net/~vlivanov/misc/antidep/02_ea_split_unique_types_01.png 
>>
>>
>> One detail is missing in the original description: there's another 
>> AllocateArray (#311) dominating the ArrayCopy (#389) and loads access 
>> it directly.
>>
>> ArrayCopy uses #311 as destination, so ArrayCopyNode::may_modify() 
>> returns true and stops further analysis:
>>
>>
>> http://hg.openjdk.java.net/jdk/hs/file/edb65305d3ac/src/hotspot/share/opto/escape.cpp#l2705 
>>
>>
>>> So it is really some problem in step 2) in EA. Could be because only 
>>> one alias index (memory slice) is used for whole array access.
>>
>> Unlikely, since I don't see any interference between accesses to 
>> different elements during split_unique_types().
>>
>>> So what memory slice of Merge node (#379) was updated to bypass membar?
>>
>> It updates instance memory slice corresponding to:
>>    bool[int:8]:NotNull:exact+any *,iid=311
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>
>>> On 3/2/18 6:47 AM, Vladimir Ivanov wrote:
>>>> Hi,
>>>>
>>>> I'm seeing unschedulable graph being produced during GCM when adding 
>>>> anti-dependence to a load node with a control dependency. I found 
>>>> the root cause, but can't decide how to fix it.
>>>>
>>>> Here are steps which lead to the broken graph:
>>>>
>>>>   (1) The load causing problems (#391) is added as part of 
>>>> specializing ArrayCopy for small arrays (added as part of 
>>>> JDK-6912521 [1] in 9). Both control & memory are tied to 
>>>> AllocateArray. (IR [2])
>>>>
>>>>   (2) EA proves that AllocateArray (#363, destination) is scalar 
>>>> replaceable and during split_unique_types() updates corresponding 
>>>> MemoryMerge (#379) and it allows to directly use memory produced by 
>>>> ArrayCopy (#255, source) bypassing the allocation & membar (#348). 
>>>> (IR [3])
>>>>
>>>>   (3) After allocation elimination, the load control dependency is 
>>>> switched to MemBarCPUOrder (#348) which was immediate dominator of 
>>>> eliminated allocation (IR [4])
>>>>
>>>>   (4) After matching the load has control on the membar, but not 
>>>> memory (IR before [5] and after [6] matching.)
>>>>
>>>>   (5) During GCM, anti-dependence from membar (#317) to the load is 
>>>> added, but it makes the graph unschedulable which then triggers the 
>>>> assertion [7] during LCM.
>>>>
>>>> Relevant places in the code: [8]
>>>>
>>>> Everything looks fine, except updates of MergeMems in step #2:
>>>>
>>>>    * the load is pinned to the proper branch after deciding what 
>>>> direction to go;
>>>>
>>>>    * wide membars do need anti-dependences on loads
>>>>
>>>> So, as a fix I'd disable memory edge updates which bypass any 
>>>> membars. Does it sound reasonable or am I missing something important?
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-6912521
>>>>
>>>> [2] http://cr.openjdk.java.net/~vlivanov/misc/antidep/01_initial.png
>>>>
>>>> [3] 
>>>> http://cr.openjdk.java.net/~vlivanov/misc/antidep/02_ea_split_unique_types.png 
>>>>
>>>>
>>>> [4] 
>>>> http://cr.openjdk.java.net/~vlivanov/misc/antidep/03_after_alloc_elimination.png 
>>>>
>>>>
>>>> [5] 
>>>> http://cr.openjdk.java.net/~vlivanov/misc/antidep/04_before_matching.png 
>>>>
>>>>
>>>> [6] 
>>>> http://cr.openjdk.java.net/~vlivanov/misc/antidep/05_after_matching.png
>>>>
>>>> [7]
>>>> #  Internal Error 
>>>> (/Users/vlivanov/ws/jdk/panama-dev/open/src/hotspot/share/opto/lcm.cpp:1169), 
>>>> pid=90414, tid=14851
>>>> #  assert(false) failed: graph should be schedulable
>>>>
>>>>
>>>> [8] http://cr.openjdk.java.net/~vlivanov/misc/antidep/webrev/