RFR(xs): 8221375: Windows 32bit build (VS2017) broken

[David Holmes]

Hi Thomas,

A few queries, comments and concerns ...

On 25/03/2019 6:58 am, Thomas Stüfe wrote:
> Hi all,
> 
> After a long time I tried to build a Windows 32bit VM (VS2017) and failed;

I'm somewhat surprised as I thought someone was actively doing Windows 
32-bit builds out there, plus there are shared code changes that should 
also have been caught by non-Windows 32-bit builds. :(

> multiple errors and warnings. Lets reverse the bitrot:
> 
> cr:
> http://cr.openjdk.java.net/~stuefe/webrevs/8221375--windows-32bit-build-(vs2017)-broken-in-many-places/webrev.00/webrev/
> 
> Issue: https://bugs.openjdk.java.net/browse/JDK-8221375
> 
> Most of the fixes are trivial: Calling convention mismatches (awt DTRACE
> callbacks), printf specifiers etc.
> 
> Had to supress a warning in os_windows_x86.cpp - I was surprised by this
> since this did not look 32bit specifc, do we disable warnings on Windows
> 64bit builds?

What version of VS2017? We use VS2017 15.9.6 and we don't disable warnings.

> The error I had in vmStructs.cpp was a bit weird: compiler complained about
> an assignment of an enum value defined like this:
> 
> hash_mask_in_place       = (address_word)hash_mask << hash_shift
> 
> to an uint64_t variable, complaining about narrowing. I did not find out
> what his problem was. In the end, I decided to add an explicit cast to
> GENERATE_VM_LONG_CONSTANT_ENTRY(name) (see vmStructs.hpp).

Not at all sure that's the right fix. In markOop.hpp we see that value 
gets special treatment on Windows-x64:

#ifndef _WIN64
          ,hash_mask               = right_n_bits(hash_bits),
          hash_mask_in_place       = (address_word)hash_mask << hash_shift
#endif
   };

   // Alignment of JavaThread pointers encoded in object header required 
by biased locking
   enum { biased_lock_alignment    = 2 << (epoch_shift + epoch_bits)
   };

#ifdef _WIN64
     // These values are too big for Win64
     const static uintptr_t hash_mask = right_n_bits(hash_bits);
     const static uintptr_t hash_mask_in_place  =
                             (address_word)hash_mask << hash_shift;
#endif

perhaps something special is needed for Windows-x86. I'm unclear how the 
values can be "too big" ??

> 
> With this patch we can build with warnings enabled on 32bit and 64bit
> windows.
> 
> Since patch fixes both hotspot and JDK parts, I'm sending it to hs-dev and
> core-libs-dev.

Don't see anything that actually comes under core-libs-dev. Looks like 
one net-dev, one awt-dev and one security-dev. Sorry.

Cheers,
David
-----

> Thanks, Thomas
>