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 build-dev mailing list