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

Reply via email to