Vladimir Kozlov via llvm-dev
2018-Jul-23 16:45 UTC
[llvm-dev] error: ordered comparison between pointer and zero ('address' (aka 'unsigned char *') and 'int')
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 >>>> >> >> >
Leslie Zhai via llvm-dev
2018-Jul-30 01:38 UTC
[llvm-dev] error: ordered comparison between pointer and zero ('address' (aka 'unsigned char *') and 'int')
Hi Vladimir, Sorry for my late response! I just came back from vacation with my wife and child :) 在 2018年07月24日 00:45, Vladimir Kozlov 写道:> 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.Thanks!> > > Reviewed. > > Please, when sending RFA ( approval request) use original 8174050 bug id.Please sponsor it, thanks a lot! http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-July/007724.html> > 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 >>>>> >>> >>> >> >-- Regards, Leslie Zhai
Thomas Schatzl via llvm-dev
2018-Jul-30 11:10 UTC
[llvm-dev] error: ordered comparison between pointer and zero ('address' (aka 'unsigned char *') and 'int')
Hi Leslie, pushed. See https://bugs.openjdk.java.net/browse/JDK-8208494 . Thomas On Mon, 2018-07-30 at 09:38 +0800, Leslie Zhai wrote:> Hi Vladimir, > > Sorry for my late response! I just came back from vacation with my > wife > and child :) > > > 在 2018年07月24日 00:45, Vladimir Kozlov 写道: > > 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. > > Thanks! > > > > > > > Reviewed. > > > > Please, when sending RFA ( approval request) use original 8174050 > > bug id. > > Please sponsor it, thanks a lot! > http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-July/007724.htm > l > > > > > 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.n > > > et/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/12462 > > > > > > 0.html > > > > > > > > > > > > > > > >