RFR: 8177346: hotspot change for 8176513 breaks jdk9 build on Ubuntu 16.04

[Vladimir Kozlov]

Good. I approved the fix for JDK 9.

Thanks,
Vladimir

On 3/21/17 2:01 PM, Vladimir Ivanov wrote:
> Looks good.
>
> (CCed hotspot-compiler-dev)
>
> Best regards,
> Vladimir Ivanov
>
> On 3/21/17 11:50 PM, David Holmes wrote:
>> Fix is trivial but this is a compiler file and should have been reviewed
>> on the hotspot-compiler-dev mailing list. There is no logical reason why
>> 8176513 should have triggered this - so seems like a GCC bug to me that
>> it wasn't flagged much earlier.
>>
>> Anyway, Reviewed.
>>
>> Thanks,
>> David
>>
>> On 22/03/2017 6:40 AM, Phil Race wrote:
>>> I am not blaming that change as being "bad" in any way.
>>> I don't see how anyone could have foreseen this side effect.
>>>
>>> -phil.
>>>
>>> On 3/21/2017 12:56 PM, Daniel D. Daugherty wrote:
>>>> Adding folks involved with the fix for 8176513.
>>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>> On 3/21/17 1:40 PM, Phil Race wrote:
>>>>> Please review a small JDK 9 build fix for a build failure due to
>>>>>
>>>>> hotspot/src/share/vm/opto/library_call.cpp:2578:3: error:
>>>>> ‘need_mem_bar’ may be used uninitialized in this function
>>>>> [-Werror=maybe-uninitialized]
>>>>>    if (need_mem_bar) insert_mem_bar(Op_MemBarCPUOrder);
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8177346
>>>>>
>>>>> I can't explain why GCC 5.4 woke up and decided to start reporting
>>>>> this warning when
>>>>> nothing logically changed that would trigger it. However I think it
>>>>> should already have
>>>>> been triggered -  it is clear to me that the compiler has a point.
>>>>>
>>>>> The fix is simple and provided in-line below :-
>>>>> ---
>>>>>
>>>>> hg diff src/share/vm/opto/library_call.cpp
>>>>> diff --git a/src/share/vm/opto/library_call.cpp
>>>>> b/src/share/vm/opto/library_call.cpp
>>>>> --- a/src/share/vm/opto/library_call.cpp
>>>>> +++ b/src/share/vm/opto/library_call.cpp
>>>>> @@ -2372,7 +2372,7 @@
>>>>>    // the barriers get omitted and the unsafe reference begins to
>>>>> "pollute"
>>>>>    // the alias analysis of the rest of the graph, either
>>>>> Compile::can_alias
>>>>>    // or Compile::must_alias will throw a diagnostic assert.)
>>>>> -  bool need_mem_bar;
>>>>> +  bool need_mem_bar = false;
>>>>>    switch (kind) {
>>>>>        case Relaxed:
>>>>>            need_mem_bar = mismatched && !adr_type->isa_aryptr();
>>>>>
>>>>> ---
>>>>>
>>>>> In fact this initialisation pattern is already used in other nearly
>>>>> identical cases in
>>>>> the same function.
>>>>> eg this one about 20 lines before my update
>>>>>   bool mismatched = false;
>>>>>
>>>>> and this one about 20 lines after ..
>>>>>   bool requires_atomic_access = false;
>>>>>
>>>>> So there is ample precedent.
>>>>>
>>>>> I will get the appropriate RDP2 approvals to push once code review is
>>>>> complete.
>>>>> I also anticipate that by the time this happens I will need to push
>>>>> it to
>>>>> http://hg.openjdk.java.net/jdk9/dev/hotspot/
>>>>> as per the email here :
>>>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-March/026155.html
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Let me know if you think otherwise.
>>>>>
>>>>> JPRT is being used to build/test the fix.
>>>>>
>>>>> -phil.
>>>>>
>>>>
>>>