RFR: 8254072: AArch64: Get rid of --disable-warnings-as-errors on Windows+ARM64 build

[Andrew Haley]

On 06/10/2020 19:17, Bernhard Urban-Forster wrote:
> I organized this PR so that each commit contains the warning emitted by MSVC as commit message and its relevant fix.
>
> Verified on
> * Linux+ARM64: `{hotspot,jdk,langtools}:tier1`, no failures.
> * Windows+ARM64: `{hotspot,jdk,langtools}:tier1`, no (new) failures.
> * internal macOS+ARM64 port: build without `--disable-warnings-as-errors` still works. Just mentioning this here, because
>   it's yet another toolchain (Xcode / clang) that needs to be kept happy [going
>   forward](https://openjdk.java.net/jeps/391).

Some of these don't look right.

@@ -69,7 +69,7 @@ int compare_immediate_pair(const void *i1, const void *i2)
 // for i = 1, ... N result<i-1> = 1 other bits are zero
 static inline uint64_t ones(int N)
 {
-  return (N == 64 ? -1ULL : (1ULL << N) - 1);
+  return (N == 64 ? ~0 : (1ULL << N) - 1);
 }

Turns out this does work because ~0 is a signed quantity which is then
sign extended to 64 bits, then converted to unsigned. But his is
obscure and therefore risky coding style, worse that what it replaces.

IMO this warning:

 warning C4146: unary minus operator applied to unsigned type, result still unsigned

should not be used. There is nothing wrong with negating an unsigned
value: doing so is well defined in all cases. Do the authors of MSVC
not understand the language? Or do they think their users do not
understand the language?

Please have a look to see how many of these diffs would go away with
that particular warning disabled.

@@ -1524,7 +1524,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,

   // Generate stack overflow check
   if (UseStackBanging) {
-    __ bang_stack_with_offset(JavaThread::stack_shadow_zone_size());
+    __ bang_stack_with_offset((int)JavaThread::stack_shadow_zone_size());
   } else {
     Unimplemented();

Could this one be fixed by changing stack_shadow_zone_size() or
bang_stack_with_offset() ? I would have thought that whatever type
stack_shadow_zone_size() returns should be compatible with
bang_stack_with_offset().

@@ -1309,7 +1309,7 @@ class StubGenerator: public StubCodeGenerator {
       __ ldrw(r16, Address(a, rscratch2, Address::lsl(exact_log2(size))));
       __ decode_heap_oop(temp); // calls verify_oop
     }
-    __ add(rscratch2, rscratch2, size);
+    __ add(rscratch2, rscratch2, (int)size);
     __ b(loop);
     __ bind(end);

Definitely not.

@@ -1367,7 +1367,7 @@ class StubGenerator: public StubCodeGenerator {
       // UnsafeCopyMemory page error: continue after ucm
       bool add_entry = !is_oop && (!aligned || sizeof(jlong) == size);
       UnsafeCopyMemoryMark ucmm(this, add_entry, true);
-      copy_memory(aligned, s, d, count, rscratch1, size);
+      copy_memory(aligned, s, d, count, rscratch1, (int)size);
     }

Better to fix the type of size. The problem seems to be that it's
passed as a size_t to generate_conjoint_copy() but it's used as an int
elsewhere.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671