RFR: 8174050: Compilation errors with clang-4.0

[Martin Buchholz]

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