Hi, Please review this set of fixes to building hotspot with GCC 8.3 or Clang 7.0 on AArch64.
Bug: https://bugs.openjdk.java.net/browse/JDK-8224851 Webrev: http://cr.openjdk.java.net/~ngasson/8224851/webrev.0/ Ran jtreg with no new failures. This first one is with GCC 8.3: /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp: In function 'void pf(long unsigned int, long unsigned int, long unsigned int, long unsigned int, long unsigned int)': /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp:775:35: error: 'void* memcpy(void*, const void*, size_t)' writing to an object of type 'class RegisterMap' with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Werror=class-memaccess] memcpy(reg_map, &map, sizeof map); ^ RegisterMap is non-POD because its base class AllocatedObj has virtual methods, so we need to use copy-assignment rather than memcpy. But because we're allocating reg_map with os::malloc we ought to use placement new to properly construct it first. But we can't do that because RegisterMap inherits from StackObj which disallows new. So I just put in some casts to void * to silence the warning. I can't see where `pf' and `internal_pf' are used - perhaps they can be called manually from the debugger? If no one uses them maybe they should be deleted? The remaining warnings / errors are with Clang 7.0. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:279:22: error: & has lower precedence than ==; == will be evaluated first [-Werror,-Wparentheses] assert_cond(bits & mask == mask); ^~~~~~~~~~~~~~ To preserve the behaviour we should write `bits & (mask == mask)' but I don't think this was what was intended so I changed it to `(bits & mask) == mask'. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/interp_masm_aarch64.hpp:44:25: error: redeclaration of using declaration using MacroAssembler::call_VM_leaf_base; ~~~~~~~~~~~~~~~~^ This using declaration appears twice in a row. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/aarch64.ad:13866:80: error: invalid suffix 'D' on floating constant __ fcmps(as_FloatRegister(opnd_array(1)->reg(ra_,this,idx1)/* src1 */), 0.0D); ^ The "d" or "D" suffix on floating point literals is not in C++98. I believe it comes from an extension to C to support decimal floating point? Anyway it's not necessary here. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp:429:66: error: implicit conversion of NULL constant to 'bool' [-Werror,-Wnull-conversion] arithmetic_op_fpu(x->op(), reg, left.result(), right.result(), NULL); ~~~~~~~~~~~~~~~~~ ^~~~ false The NULL here is for the `bool is_strictfp' argument to LIRGenerator::arithmetic_op_fpu. Changing it to `false' would preserve the existing behaviour but it seems like it should be `x->is_strictfp()' to match other platforms. As a consequence of this we need to handle the lir_{div,mul}_scriptfp opcodes in LIR_Assembler::arith_op, but these just fall through to the non-strictfp variants so there should be no behaviour change. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp:1074:24: error: '&&' within '||' [-Werror,-Wlogical-op-parentheses] if (is_unordered && op->cond() == lir_cond_equal ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Added parenthesis to make this explicit. No behaviour change. /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/copy_linux_aarch64.s:162:32: error: index must be a multiple of 8 in range [0, 32760]. prfm pldl1keep, [s, #-256] ^ The immediate offset in `prfm' gets zero-extended to 64-bits and then left shifted three places so can only be unsigned. GNU as actually assembles [s, #-256] the same as [s]: 0: f9800000 prfm pldl1keep, [x0] Seems like GNU as ought to report an error here instead. I've replaced this with `prfum' which has a sign-extended offset. /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:43:39: error: cannot initialize a parameter of type 'char *' with an lvalue of type 'unsigned long' return __sync_add_and_fetch(dest, add_value); ^~~~~~~~~ If the add_and_fetch template is instantiated with I=integer and D=pointer then we need an explicit cast here. /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp:679:8: error: conflicting types for '_Copy_conjoint_jshorts_atomic' void _Copy_conjoint_jshorts_atomic(jshort* from, jshort* to, size_t count) { ^ This family of functions is declared with const `from' pointer but in the definition it is non-const. Made it const everywhere. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp:177:13: error: using the result of an assignment as a condition without parentheses [-Werror,-Wparentheses] if (p = strchr(buf, ':')) { ~~^~~~~~~~~~~~~~~~~~ Put parentheses around the argument. No functional change. /home/nicgas01/jdk/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp:2684:17: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value] offset &= -1<<12; ~~^ Rewrote as ~((1<<12) - 1) which matches the masks used in the rest of the function. /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp:103:20: error: variable 'esp' is uninitialized when used here [-Werror,-Wuninitialized] return (address) esp; ^~~ /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp:101:21: note: initialize the variable 'esp' to silence this warning register void *esp __asm__ (SPELL_REG_SP); ^ = nullptr We could silence the -Wuninitialized warning for this function, but that doesn't help much because Clang then compiles os::current_stack_pointer() into: 0000000000000000 <_ZN2os21current_stack_pointerEv>: 0: d65f03c0 ret Whereas with GCC we get: 0000000000000250 <_ZN2os21current_stack_pointerEv>: 250: 910003e0 mov x0, sp 254: d65f03c0 ret Added some inline assembly to explicitly move sp into the result #ifdef __clang__. Same with _get_previous_fp(). Thanks, Nick
