RFR: 8174050: Compilation errors with clang-4.0

[Vladimir Kozlov]

Yes, it should have ->_lo. Sorry about that.

I am finishing testing with updated assert (and with ->_lo). No new 
failures so far.

Thanks,
Vladimir

On 8/24/17 10:23 AM, Martin Buchholz wrote:
> Vladimir, did you mean (reinserting ->_lo)
> 
> assert(rng->Opcode() == Op_LoadRange || iff->is_RangeCheck() ||
> _igvn.type(rng)->is_int()->_lo >= 0, "must be");
> 
> On Wed, Aug 23, 2017 at 9:16 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com
>> wrote:
> 
>> I also hit this failure.
>>
>> The code which guards this assert has additional condition "&&
>> !iff->is_RangeCheck()" due to which no check for positive value happened:
>>
>> http://hg.openjdk.java.net/jdk10/hs/hotspot/file/821ef7c1008
>> 5/src/share/vm/opto/loopPredicate.cpp#l590
>>
>> I think we should check for RangeCheck node in assert:
>>
>> assert(rng->Opcode() == Op_LoadRange || iff->is_RangeCheck() ||
>> _igvn.type(rng)->is_int() >= 0, "must be");
>>
>> RangeCheck has dynamic check that range is positive.
>> I think it should be safe to relax this assert.
>>
>> Thanks,
>> Vladimir
>>
>>
>> On 8/23/17 7:33 PM, Martin Buchholz wrote:
>>
>>> during ad hoc testing I saw the crash below, so this assert needs more
>>> work
>>> (but great job on the error reporting here!)
>>>
>>> [2017-08-23 19:29:22,374] Agent[0]: stdout: # To suppress the following
>>> error report, specify this argument
>>> [2017-08-23 19:29:22,390] Agent[0]: stdout: # after -XX: or in .hotspotrc:
>>>    SuppressErrorAt=/loopPredicate.cpp:915
>>> [2017-08-23 19:29:22,391] Agent[0]: stdout: #
>>> [2017-08-23 19:29:22,392] Agent[0]: stdout: # A fatal error has been
>>> detected by the Java Runtime Environment:
>>> [2017-08-23 19:29:22,399] Agent[0]: stdout: #
>>> [2017-08-23 19:29:22,400] Agent[0]: stdout: #  Internal Error
>>> (/home/martin/ws/jdk10-clang-4.0/hotspot/src/share/vm/opto/l
>>> oopPredicate.cpp:915),
>>> pid=31347, tid=31367
>>> [2017-08-23 19:29:22,400] Agent[0]: stdout: #  assert(rng->Opcode() ==
>>> Op_LoadRange || _igvn.type(rng)->is_int()->_lo >= 0) failed: must be
>>>
>>>
>>> On Wed, Aug 23, 2017 at 5:48 PM, Vladimir Kozlov <
>>> vladimir.kozlov at oracle.com
>>>
>>>> wrote:
>>>>
>>>
>>> I submitted pre-integration testing.
>>>>
>>>> Vladimir
>>>>
>>>>
>>>> On 8/23/17 5:02 PM, Martin Buchholz wrote:
>>>>
>>>> I assume I need a sponsor; if so, please import
>>>>> http://cr.openjdk.java.net/~martin/webrevs/openjdk10/pointer
>>>>> -sign-comparison/pointer-sign-comparison.patch
>>>>>
>>>>> On Wed, Aug 23, 2017 at 4:46 PM, Vladimir Kozlov <
>>>>> vladimir.kozlov at oracle.com
>>>>>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>> Looks good to me.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 8/23/17 4:37 PM, Martin Buchholz wrote:
>>>>>>
>>>>>> Thanks, Vladimir.
>>>>>>
>>>>>>> Webrev regenerated.
>>>>>>>
>>>>>>> I blindly coded
>>>>>>>
>>>>>>> - assert(rng->Opcode() == Op_LoadRange || _igvn.type(rng)->is_int() >=
>>>>>>> 0,
>>>>>>> "must be");
>>>>>>> + assert(rng->Opcode() == Op_LoadRange ||
>>>>>>> _igvn.type(rng)->is_int()->_lo
>>>>>>>
>>>>>>> = 0, "must be");
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Aug 23, 2017 at 3:39 PM, Vladimir Kozlov <
>>>>>>> vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>        Thank you, Martin
>>>>>>>
>>>>>>>        Yes, sign compare is bad for pointers.
>>>>>>>
>>>>>>>        Note, for pointers compare with use NULL instead of 0. Please,
>>>>>>> use
>>>>>>> NULL.
>>>>>>>
>>>>>>>        The assert check in loopPredicate.cpp is simple missing
>>>>>>> reference
>>>>>>> to
>>>>>>>        field _lo (low bound of values range):
>>>>>>>
>>>>>>>          _igvn.type(rng)->is_int()->_lo >= 0
>>>>>>>
>>>>>>>        Please, fix it this way.
>>>>>>>
>>>>>>>        Thanks,
>>>>>>>        Vladimir
>>>>>>>
>>>>>>>
>>>>>>>        On 8/23/17 2:25 PM, Martin Buchholz wrote:
>>>>>>>
>>>>>>>            https://bugs.openjdk.java.net/browse/JDK-8174050
>>>>>>>            <https://bugs.openjdk.java.net/browse/JDK-8174050>
>>>>>>>            http://cr.openjdk.java.net/~ma
>>>>>>> rtin/webrevs/openjdk10/pointer
>>>>>>> -sign-comparison/
>>>>>>>            <http://cr.openjdk.java.net/~m
>>>>>>> artin/webrevs/openjdk10/pointe
>>>>>>> r-sign-comparison/>
>>>>>>>
>>>>>>>            My webrev could go in as is, but better would be for
>>>>>>> someone to
>>>>>>>            figure out
>>>>>>>            the intent of the nonsensical assert in
>>>>>>>
>>>>>>>            src/share/vm/opto/loopPredicate.cpp
>>>>>>>
>>>>>>>            (There is more to be done to support clang, but this is
>>>>>>> enough
>>>>>>>            to build
>>>>>>>            openjdk without patching source)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>