llvm dev - Jul 2018 - error: ordered comparison between pointer and zero ('address' (aka 'unsigned char *') and 'int')

Hi Thomas,

Thanks for your kind response!

Please review my backport for hs25, thanks a lot!



diff -r 3544d85cfe11 src/share/vm/opto/lcm.cpp
--- a/src/share/vm/opto/lcm.cpp    Thu Jul 19 10:00:36 2018 +0100
+++ b/src/share/vm/opto/lcm.cpp    Fri Jul 20 10:06:37 2018 +0800
@@ -49,7 +49,7 @@
  // Check whether val is not-null-decoded compressed oop,
  // i.e. will grab into the base of the heap if it represents NULL.
  static bool accesses_heap_base_zone(Node *val) {
-  if (Universe::narrow_oop_base() > 0) { // Implies UseCompressedOops.
+  if (Universe::narrow_oop_base() != NULL) { // Implies UseCompressedOops.
      if (val && val->is_Mach()) {
        if (val->as_Mach()->ideal_Opcode() == Op_DecodeN) {
          // This assumes all Decodes with TypePtr::NotNull are matched 
to nodes that
diff -r 3544d85cfe11 src/share/vm/opto/loopPredicate.cpp
--- a/src/share/vm/opto/loopPredicate.cpp    Thu Jul 19 10:00:36 2018 +0100
+++ b/src/share/vm/opto/loopPredicate.cpp    Fri Jul 20 10:06:37 2018 +0800
@@ -869,7 +869,7 @@
        Node*          idx    = cmp->in(1);
        assert(!invar.is_invariant(idx), "index is variant");
        Node* rng = cmp->in(2);
-      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");
        assert(invar.is_invariant(rng), "range must be invariant");
        int scale    = 1;
        Node* offset = zero;
diff -r 3544d85cfe11 src/share/vm/runtime/virtualspace.cpp
--- a/src/share/vm/runtime/virtualspace.cpp    Thu Jul 19 10:00:36 2018 
+0100
+++ b/src/share/vm/runtime/virtualspace.cpp    Fri Jul 20 10:06:37 2018 
+0800
@@ -342,7 +342,7 @@
                  (UseCompressedOops && (Universe::narrow_oop_base() != 
NULL) &&
                   Universe::narrow_oop_use_implicit_null_checks()) ?
                    lcm(os::vm_page_size(), alignment) : 0) {
-  if (base() > 0) {
+  if (base() != NULL) {
      MemTracker::record_virtual_memory_type((address)base(), mtJavaHeap);
    }




在 2018年07月19日 19:37, Thomas Schatzl 写道:
> Hi,
>
> On Thu, 2018-07-19 at 18:56 +0800, Leslie Zhai wrote:
>> Hi HotSpot and LLVM developers,
>>
>> I am building OpenJDK8[1] with LLVM toolchain[2] for mips64el, it
>> failed to build:
>>
>>
>> /home/loongson/jdk8-mips/hotspot/src/share/vm/opto/lcm.cpp:52:35:
>> error:
>> ordered comparison between pointer and zero ('address' (aka
>> 'unsigned
>> char *') and 'int')
>>     if (Universe::narrow_oop_base() > 0) { // Implies
>> UseCompressedOops.
>>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
>>
>>
>> Just like compiling Linux kernel with LLVM for mips64el[3], the bug
>> could be fixed both in Compiler side and HotSpot side, please give
>> me some suggestion, thanks a lot! And my sincere thanks will go to
>> Nick who helped me a lot!
>    this has been fixed in the jvm in JDK10 with https://bugs.openjdk.jav
> a.net/browse/JDK-8174050 .
>
> It has not been backported to jdk8; others may be able to tell you if
> it can/will be backported.
>
> Thanks,
>    Thomas
>
>> 1. http://hg.loongnix.org/
>> 2. Loongson clang version 7.0.0
>> (git at github.com:Loong-Language/loong-clang.git
>> c36069cffc57a30a20782bf327a87bed4e48a6c2)
>> (git at github.com:Loong-Language/loong-llvm.git
>> 59cb663e72874dda740aa2b18bf47ba65b32fe9b) (based on LLVM 7.0.0svn)
>> Target: mips64el-redhat-linux
>> Thread model: posix
>> InstalledDir: /opt/loong-llvm/bin
>> Found candidate GCC installation: /usr/lib/gcc/mips64el-redhat-
>> linux/4.9.3
>> Selected GCC installation: /usr/lib/gcc/mips64el-redhat-linux/4.9.3
>> Candidate multilib: .;
>> Selected multilib: .;
>> 3. http://lists.llvm.org/pipermail/llvm-dev/2018-July/124620.html
>>
-- 
Regards,
Leslie Zhai

Hi Leslie,

On Fri, 2018-07-20 at 10:07 +0800, Leslie Zhai wrote:
> Hi Thomas,
> 
> Thanks for your kind response!
> 
> Please review my backport for hs25, thanks a lot!
> 
  I can't review the second hunk because it is out of my area of
expertise and it does not look a trivial change either. The other hunks
look good. Somebody else more proficient with the compiler needs to
look at this.

The further process for 8u fixes is then to send a request for approval
email to jdk8u-dev at openjdk.java.net with some information about the
original bug and this review; see e.g. http://mail.openjdk.java.net/pip
ermail/jdk8u-dev/2018-July/007708.html for an example.

If you need a sponsor, I could do that for you after approval (I am onjdk8u-dev,
so I would notice :) ).

Thanks,
  Thomas
> 
> 
> diff -r 3544d85cfe11 src/share/vm/opto/lcm.cpp
> --- a/src/share/vm/opto/lcm.cpp    Thu Jul 19 10:00:36 2018 +0100
> +++ b/src/share/vm/opto/lcm.cpp    Fri Jul 20 10:06:37 2018 +0800
> @@ -49,7 +49,7 @@
>   // Check whether val is not-null-decoded compressed oop,
>   // i.e. will grab into the base of the heap if it represents NULL.
>   static bool accesses_heap_base_zone(Node *val) {
> -  if (Universe::narrow_oop_base() > 0) { // Implies
> UseCompressedOops.
> +  if (Universe::narrow_oop_base() != NULL) { // Implies
> UseCompressedOops.
>       if (val && val->is_Mach()) {
>         if (val->as_Mach()->ideal_Opcode() == Op_DecodeN) {
>           // This assumes all Decodes with TypePtr::NotNull are
> matched 
> to nodes that
> diff -r 3544d85cfe11 src/share/vm/opto/loopPredicate.cpp
> --- a/src/share/vm/opto/loopPredicate.cpp    Thu Jul 19 10:00:36 2018
> +0100
> +++ b/src/share/vm/opto/loopPredicate.cpp    Fri Jul 20 10:06:37 2018
> +0800
> @@ -869,7 +869,7 @@
>         Node*          idx    = cmp->in(1);
>         assert(!invar.is_invariant(idx), "index is variant");
>         Node* rng = cmp->in(2);
> -      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");
>         assert(invar.is_invariant(rng), "range must be
invariant");
>         int scale    = 1;
>         Node* offset = zero;
> diff -r 3544d85cfe11 src/share/vm/runtime/virtualspace.cpp
> --- a/src/share/vm/runtime/virtualspace.cpp    Thu Jul 19 10:00:36
> 2018 
> +0100
> +++ b/src/share/vm/runtime/virtualspace.cpp    Fri Jul 20 10:06:37
> 2018 
> +0800
> @@ -342,7 +342,7 @@
>                   (UseCompressedOops &&
(Universe::narrow_oop_base()
> != 
> NULL) &&
>                    Universe::narrow_oop_use_implicit_null_checks()) ?
>                     lcm(os::vm_page_size(), alignment) : 0) {
> -  if (base() > 0) {
> +  if (base() != NULL) {
>       MemTracker::record_virtual_memory_type((address)base(),
> mtJavaHeap);
>     }
> 
> 
> 
> 
> 在 2018年07月19日 19:37, Thomas Schatzl 写道:
> > Hi,
> > 
> > On Thu, 2018-07-19 at 18:56 +0800, Leslie Zhai wrote:
> > > Hi HotSpot and LLVM developers,
> > > 
> > > I am building OpenJDK8[1] with LLVM toolchain[2] for mips64el, it
> > > failed to build:
> > > 
> > > 
> > > /home/loongson/jdk8-mips/hotspot/src/share/vm/opto/lcm.cpp:52:35:
> > > error:
> > > ordered comparison between pointer and zero ('address'
(aka
> > > 'unsigned
> > > char *') and 'int')
> > >     if (Universe::narrow_oop_base() > 0) { // Implies
> > > UseCompressedOops.
> > >         ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
> > > 
> > > 
> > > Just like compiling Linux kernel with LLVM for mips64el[3], the
> > > bug
> > > could be fixed both in Compiler side and HotSpot side, please
> > > give
> > > me some suggestion, thanks a lot! And my sincere thanks will go
> > > to
> > > Nick who helped me a lot!
> > 
> >    this has been fixed in the jvm in JDK10 with https://bugs.openjd
> > k.jav
> > a.net/browse/JDK-8174050 .
> > 
> > It has not been backported to jdk8; others may be able to tell you
> > if
> > it can/will be backported.
> > 
> > Thanks,
> >    Thomas
> > 
> > > 1. http://hg.loongnix.org/
> > > 2. Loongson clang version 7.0.0
> > > (git at github.com:Loong-Language/loong-clang.git
> > > c36069cffc57a30a20782bf327a87bed4e48a6c2)
> > > (git at github.com:Loong-Language/loong-llvm.git
> > > 59cb663e72874dda740aa2b18bf47ba65b32fe9b) (based on LLVM
> > > 7.0.0svn)
> > > Target: mips64el-redhat-linux
> > > Thread model: posix
> > > InstalledDir: /opt/loong-llvm/bin
> > > Found candidate GCC installation: /usr/lib/gcc/mips64el-redhat-
> > > linux/4.9.3
> > > Selected GCC installation: /usr/lib/gcc/mips64el-redhat-
> > > linux/4.9.3
> > > Candidate multilib: .;
> > > Selected multilib: .;
> > > 3. http://lists.llvm.org/pipermail/llvm-dev/2018-July/124620.html
> > > 
> 
>

Hi Thomas,

Looks good.

Your changes in loopPredicate.cpp does not match original changes - they 
miss iff->is_RangeCheck() check [1]. But in JDK8 we did not have 
specialized RangeCheckNode class in C2. Suggested fix should be fine fro 
jdk 8u.

Reviewed.

Please, when sending RFA ( approval request) use original 8174050 bug id.

Thanks,
Vladimir

[1] http://hg.openjdk.java.net/jdk10/jdk10/hotspot/rev/316854ef2fa2

On 7/23/18 8:49 AM, Thomas Schatzl wrote:
> Hi Leslie,
> 
> On Fri, 2018-07-20 at 10:07 +0800, Leslie Zhai wrote:
>> Hi Thomas,
>>
>> Thanks for your kind response!
>>
>> Please review my backport for hs25, thanks a lot!
>>
> 
>    I can't review the second hunk because it is out of my area of
> expertise and it does not look a trivial change either. The other hunks
> look good. Somebody else more proficient with the compiler needs to
> look at this.
> 
> The further process for 8u fixes is then to send a request for approval
> email to jdk8u-dev at openjdk.java.net with some information about the
> original bug and this review; see e.g. http://mail.openjdk.java.net/pip
> ermail/jdk8u-dev/2018-July/007708.html for an example.
> 
> If you need a sponsor, I could do that for you after approval (I am
onjdk8u-dev, so I would notice :) ).
> 
> Thanks,
>    Thomas
> 
>>
>>
>> diff -r 3544d85cfe11 src/share/vm/opto/lcm.cpp
>> --- a/src/share/vm/opto/lcm.cpp    Thu Jul 19 10:00:36 2018 +0100
>> +++ b/src/share/vm/opto/lcm.cpp    Fri Jul 20 10:06:37 2018 +0800
>> @@ -49,7 +49,7 @@
>>    // Check whether val is not-null-decoded compressed oop,
>>    // i.e. will grab into the base of the heap if it represents NULL.
>>    static bool accesses_heap_base_zone(Node *val) {
>> -  if (Universe::narrow_oop_base() > 0) { // Implies
>> UseCompressedOops.
>> +  if (Universe::narrow_oop_base() != NULL) { // Implies
>> UseCompressedOops.
>>        if (val && val->is_Mach()) {
>>          if (val->as_Mach()->ideal_Opcode() == Op_DecodeN) {
>>            // This assumes all Decodes with TypePtr::NotNull are
>> matched
>> to nodes that
>> diff -r 3544d85cfe11 src/share/vm/opto/loopPredicate.cpp
>> --- a/src/share/vm/opto/loopPredicate.cpp    Thu Jul 19 10:00:36 2018
>> +0100
>> +++ b/src/share/vm/opto/loopPredicate.cpp    Fri Jul 20 10:06:37 2018
>> +0800
>> @@ -869,7 +869,7 @@
>>          Node*          idx    = cmp->in(1);
>>          assert(!invar.is_invariant(idx), "index is
variant");
>>          Node* rng = cmp->in(2);
>> -      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");
>>          assert(invar.is_invariant(rng), "range must be
invariant");
>>          int scale    = 1;
>>          Node* offset = zero;
>> diff -r 3544d85cfe11 src/share/vm/runtime/virtualspace.cpp
>> --- a/src/share/vm/runtime/virtualspace.cpp    Thu Jul 19 10:00:36
>> 2018
>> +0100
>> +++ b/src/share/vm/runtime/virtualspace.cpp    Fri Jul 20 10:06:37
>> 2018
>> +0800
>> @@ -342,7 +342,7 @@
>>                    (UseCompressedOops &&
(Universe::narrow_oop_base()
>> !>> NULL) &&
>>                     Universe::narrow_oop_use_implicit_null_checks()) ?
>>                      lcm(os::vm_page_size(), alignment) : 0) {
>> -  if (base() > 0) {
>> +  if (base() != NULL) {
>>        MemTracker::record_virtual_memory_type((address)base(),
>> mtJavaHeap);
>>      }
>>
>>
>>
>>
>> 在 2018年07月19日 19:37, Thomas Schatzl 写道:
>>> Hi,
>>>
>>> On Thu, 2018-07-19 at 18:56 +0800, Leslie Zhai wrote:
>>>> Hi HotSpot and LLVM developers,
>>>>
>>>> I am building OpenJDK8[1] with LLVM toolchain[2] for mips64el,
it
>>>> failed to build:
>>>>
>>>>
>>>>
/home/loongson/jdk8-mips/hotspot/src/share/vm/opto/lcm.cpp:52:35:
>>>> error:
>>>> ordered comparison between pointer and zero ('address'
(aka
>>>> 'unsigned
>>>> char *') and 'int')
>>>>      if (Universe::narrow_oop_base() > 0) { // Implies
>>>> UseCompressedOops.
>>>>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
>>>>
>>>>
>>>> Just like compiling Linux kernel with LLVM for mips64el[3], the
>>>> bug
>>>> could be fixed both in Compiler side and HotSpot side, please
>>>> give
>>>> me some suggestion, thanks a lot! And my sincere thanks will go
>>>> to
>>>> Nick who helped me a lot!
>>>
>>>     this has been fixed in the jvm in JDK10 with
https://bugs.openjd
>>> k.jav
>>> a.net/browse/JDK-8174050 .
>>>
>>> It has not been backported to jdk8; others may be able to tell you
>>> if
>>> it can/will be backported.
>>>
>>> Thanks,
>>>     Thomas
>>>
>>>> 1. http://hg.loongnix.org/
>>>> 2. Loongson clang version 7.0.0
>>>> (git at github.com:Loong-Language/loong-clang.git
>>>> c36069cffc57a30a20782bf327a87bed4e48a6c2)
>>>> (git at github.com:Loong-Language/loong-llvm.git
>>>> 59cb663e72874dda740aa2b18bf47ba65b32fe9b) (based on LLVM
>>>> 7.0.0svn)
>>>> Target: mips64el-redhat-linux
>>>> Thread model: posix
>>>> InstalledDir: /opt/loong-llvm/bin
>>>> Found candidate GCC installation: /usr/lib/gcc/mips64el-redhat-
>>>> linux/4.9.3
>>>> Selected GCC installation: /usr/lib/gcc/mips64el-redhat-
>>>> linux/4.9.3
>>>> Candidate multilib: .;
>>>> Selected multilib: .;
>>>> 3.
http://lists.llvm.org/pipermail/llvm-dev/2018-July/124620.html
>>>>
>>
>>
>

Hi Thomas,

Sorry for my late response! I just came back from vacation with my wife 
and child :)


在 2018年07月23日 23:49, Thomas Schatzl 写道:
> Hi Leslie,
>
> On Fri, 2018-07-20 at 10:07 +0800, Leslie Zhai wrote:
>> Hi Thomas,
>>
>> Thanks for your kind response!
>>
>> Please review my backport for hs25, thanks a lot!
>>
>    I can't review the second hunk because it is out of my area of
> expertise and it does not look a trivial change either. The other hunks
> look good. Somebody else more proficient with the compiler needs to
> look at this.
>
> The further process for 8u fixes is then to send a request for approval
> email to jdk8u-dev at openjdk.java.net with some information about the
> original bug and this review; see e.g. http://mail.openjdk.java.net/pip
> ermail/jdk8u-dev/2018-July/007708.html for an example.
>
> If you need a sponsor, I could do that for you after approval (I am
onjdk8u-dev, so I would notice :) ).
Thanks for your teaching! I sent a RFA email to 
jdk8u-dev at openjdk.java.net with the information about the original bug 
and the review by following the example.
Please sponsor it, thanks a lot!
http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-July/007724.html
>
> Thanks,
>    Thomas
>
>>
>> diff -r 3544d85cfe11 src/share/vm/opto/lcm.cpp
>> --- a/src/share/vm/opto/lcm.cpp    Thu Jul 19 10:00:36 2018 +0100
>> +++ b/src/share/vm/opto/lcm.cpp    Fri Jul 20 10:06:37 2018 +0800
>> @@ -49,7 +49,7 @@
>>    // Check whether val is not-null-decoded compressed oop,
>>    // i.e. will grab into the base of the heap if it represents NULL.
>>    static bool accesses_heap_base_zone(Node *val) {
>> -  if (Universe::narrow_oop_base() > 0) { // Implies
>> UseCompressedOops.
>> +  if (Universe::narrow_oop_base() != NULL) { // Implies
>> UseCompressedOops.
>>        if (val && val->is_Mach()) {
>>          if (val->as_Mach()->ideal_Opcode() == Op_DecodeN) {
>>            // This assumes all Decodes with TypePtr::NotNull are
>> matched
>> to nodes that
>> diff -r 3544d85cfe11 src/share/vm/opto/loopPredicate.cpp
>> --- a/src/share/vm/opto/loopPredicate.cpp    Thu Jul 19 10:00:36 2018
>> +0100
>> +++ b/src/share/vm/opto/loopPredicate.cpp    Fri Jul 20 10:06:37 2018
>> +0800
>> @@ -869,7 +869,7 @@
>>          Node*          idx    = cmp->in(1);
>>          assert(!invar.is_invariant(idx), "index is
variant");
>>          Node* rng = cmp->in(2);
>> -      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");
>>          assert(invar.is_invariant(rng), "range must be
invariant");
>>          int scale    = 1;
>>          Node* offset = zero;
>> diff -r 3544d85cfe11 src/share/vm/runtime/virtualspace.cpp
>> --- a/src/share/vm/runtime/virtualspace.cpp    Thu Jul 19 10:00:36
>> 2018
>> +0100
>> +++ b/src/share/vm/runtime/virtualspace.cpp    Fri Jul 20 10:06:37
>> 2018
>> +0800
>> @@ -342,7 +342,7 @@
>>                    (UseCompressedOops &&
(Universe::narrow_oop_base()
>> !>> NULL) &&
>>                     Universe::narrow_oop_use_implicit_null_checks()) ?
>>                      lcm(os::vm_page_size(), alignment) : 0) {
>> -  if (base() > 0) {
>> +  if (base() != NULL) {
>>        MemTracker::record_virtual_memory_type((address)base(),
>> mtJavaHeap);
>>      }
>>
>>
>>
>>
>> 在 2018年07月19日 19:37, Thomas Schatzl 写道:
>>> Hi,
>>>
>>> On Thu, 2018-07-19 at 18:56 +0800, Leslie Zhai wrote:
>>>> Hi HotSpot and LLVM developers,
>>>>
>>>> I am building OpenJDK8[1] with LLVM toolchain[2] for mips64el,
it
>>>> failed to build:
>>>>
>>>>
>>>>
/home/loongson/jdk8-mips/hotspot/src/share/vm/opto/lcm.cpp:52:35:
>>>> error:
>>>> ordered comparison between pointer and zero ('address'
(aka
>>>> 'unsigned
>>>> char *') and 'int')
>>>>      if (Universe::narrow_oop_base() > 0) { // Implies
>>>> UseCompressedOops.
>>>>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~
>>>>
>>>>
>>>> Just like compiling Linux kernel with LLVM for mips64el[3], the
>>>> bug
>>>> could be fixed both in Compiler side and HotSpot side, please
>>>> give
>>>> me some suggestion, thanks a lot! And my sincere thanks will go
>>>> to
>>>> Nick who helped me a lot!
>>>     this has been fixed in the jvm in JDK10 with
https://bugs.openjd
>>> k.jav
>>> a.net/browse/JDK-8174050 .
>>>
>>> It has not been backported to jdk8; others may be able to tell you
>>> if
>>> it can/will be backported.
>>>
>>> Thanks,
>>>     Thomas
>>>
>>>> 1. http://hg.loongnix.org/
>>>> 2. Loongson clang version 7.0.0
>>>> (git at github.com:Loong-Language/loong-clang.git
>>>> c36069cffc57a30a20782bf327a87bed4e48a6c2)
>>>> (git at github.com:Loong-Language/loong-llvm.git
>>>> 59cb663e72874dda740aa2b18bf47ba65b32fe9b) (based on LLVM
>>>> 7.0.0svn)
>>>> Target: mips64el-redhat-linux
>>>> Thread model: posix
>>>> InstalledDir: /opt/loong-llvm/bin
>>>> Found candidate GCC installation: /usr/lib/gcc/mips64el-redhat-
>>>> linux/4.9.3
>>>> Selected GCC installation: /usr/lib/gcc/mips64el-redhat-
>>>> linux/4.9.3
>>>> Candidate multilib: .;
>>>> Selected multilib: .;
>>>> 3.
http://lists.llvm.org/pipermail/llvm-dev/2018-July/124620.html
>>>>
>>
-- 
Regards,
Leslie Zhai