RFR: 8174050: Compilation errors with clang-4.0

[Vladimir Kozlov]

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/821ef7c10085/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/loopPredicate.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/~martin/webrevs/openjdk10/pointer
>>>>> -sign-comparison/
>>>>>           <http://cr.openjdk.java.net/~martin/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)
>>>>>
>>>>>
>>>>>
>>>>>