RFR: 8224851: AArch64: fix warnings and errors with Clang and GCC 8.3
    Kim Barrett 
    kim.barrett at oracle.com
       
    Thu Jun 13 00:45:45 UTC 2019
    
    
  
> On Jun 12, 2019, at 5:35 AM, Nick Gasson <Nick.Gasson at arm.com> wrote:
> 
> Hi Andrew,
> 
> Sorry for the delay, I've put an updated webrev here:
> 
> http://cr.openjdk.java.net/~ngasson/8224851/webrev.1/
Since I've been commenting on some parts, I decided I might as well
look at all of it.  So here's a review.  I don't need to re-review any
of the suggested changes.
------------------------------------------------------------------------------
src/hotspot/cpu/aarch64/frame_aarch64.cpp
 772     reg_map = (RegisterMap*)os::malloc(sizeof(RegisterMap), mtNone);
Use NEW_C_HEAP_OBJ(RegisterMap, mtNone)
------------------------------------------------------------------------------
src/hotspot/cpu/aarch64/macroAssembler_aarch64_trig.cpp
 384         fcmpd(v26, 0.0);                          // if NE then jx == 2. else it's 1 or 0
and 
 388         fcmpd(v7, 0.0);                           // v7 == 0 => jx = 0. Else jx = 1
EOL comments no longer aligned with others nearby.
------------------------------------------------------------------------------
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp
 180       if ((p = strchr(buf, ':'))) {
Hotspot Coding Style says not to use implicit bool, instead using
explicit comparisons, e.g. this should be
 180       if ((p = strchr(buf, ':')) != NULL) {
That would have avoided the warning in the first place.
And yes, I know there are lots of violations of that guideline.
------------------------------------------------------------------------------
src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp
  43 #ifdef __clang__
  44     D res = __atomic_add_fetch(dest, add_value, __ATOMIC_RELEASE);
  45     FULL_MEM_BARRIER;
  46     return res;
  47 #else
Is there a reason to conditionalize use of __atomic_add_fetch for
clang and continue to use __sync_add_and_fetch for gcc, rather than
using __atomic_add_fetch unconditionally?
Is __ATOMIC_RELEASE and a following FULL_MEM_BARRIER the right usage
to get the equivalent of __sync_add_and_fetch?  Or should
__ATOMIC_SEQ_CST be used?  Or should the latter be actively avoided?
I remember some discussion and questions in that area, about how to
implement memory_order_seq_cst on ARM processors, but wasn't paying
close attention since I don't deal with ARM much.
Andrew?  I'll happily defer to you here.
------------------------------------------------------------------------------
src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp
I didn't review the stuff around os::current_stack_pointer and
os::current_frame.
------------------------------------------------------------------------------
src/hotspot/os_cpu/linux_aarch64/copy_linux_aarch64.s
 162         prfum   pldl1keep, [s, #-256]
I didn't review this change.
------------------------------------------------------------------------------
    
    
More information about the hotspot-dev
mailing list