RFR(S): 8172145: C2: anti dependence missed because store hidden by membar

[Vladimir Kozlov]

Hmm, may be it is because MemBarVolatile is not listed in InstructForm::is_wide_memory_kill() in adlc/formssel.cpp.
I think we should list it in is_wide_memory_kill(). I looked on all places where it is added and all use AliasIdxBot - 
wide memory.

Vladimir

On 1/2/17 12:47 PM, Vladimir Kozlov wrote:
> I think the problem is this:
>
> "The membar's adr_type is NULL so can_alias returns false and reordering is not prevented."
>
> It should not be NULL. Type should be TypePtr::BOTTOM or specific memory related type.
>
> Loads should not be moved below membar if membar is wide (TypePtr::BOTTOM, affect all memory).
>
> It would be interesting to see these nodes in Ideal graph. What memory connections there?
>
>   if (support_IRIW_for_not_multiple_copy_atomic_cpu && field->is_volatile()) {
>     insert_mem_bar(Op_MemBarVolatile);   // StoreLoad barrier
>
> Node* GraphKit::insert_mem_bar(int opcode, Node* precedent) {
>   MemBarNode* mb = MemBarNode::make(C, opcode, Compile::AliasIdxBot, precedent);
>
> and
>
>   _alias_types[AliasIdxBot]->Init(AliasIdxBot, TypePtr::BOTTOM);
>
> So I don't get why you have "adr_type is NULL"
>
> Thanks,
> Vladimir
>
> On 12/30/16 7:34 AM, Andrew Haley wrote:
>> Hi,
>>
>> On 30/12/16 13:29, Doerr, Martin wrote:
>>>
>>> we found a C2 Compiler bug which leads to incorrect reordering of
>>> memory accesses on PPC64 due to missing anti-dependency. Details are
>>> described here: https://bugs.openjdk.java.net/browse/JDK-8172145
>>
>> Another missing anti-dependency!
>>
>>> The issue can be fixed by this webrev:
>>> http://cr.openjdk.java.net/~mdoerr/8172145_C2_antidep/webrev.00/
>>
>> Wouldn't it make more sense to calculate is_Mach_MemBarVolatile lazily?
>>
>> Andrew.
>>