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

[Vladimir Ivanov]

On 3/6/18 10:26 PM, Vladimir Kozlov wrote:
> 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?

Exactly. Anti-dependences are added from membar (#317) to the loads 
(#380/...) and it makes the graph unschedulable in LCM.

Best regards,
Vladimir Ivanov

>> 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/