Hi Alexey, thanks. I posted an updated fix for this on awt-net (8221405) since David asked me to split the patch up. Could you please review it?
Thanks! ..Thomas On Mon, Mar 25, 2019 at 3:46 PM Alexey Ivanov <[email protected]> wrote: > Hi Thomas, > > There was a thread about failing build on 32 bit Windows: > http://mail.openjdk.java.net/pipermail/build-dev/2018-November/023767.html > > It continued in February 2019: > http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024925.html > > Fast debug builds were affected. Likely there was mismatch between > CALLBACK / JNICALL macro: > http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024972.html > http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024973.html > > Magnus suggested a fix: > http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024996.html > > Yet there was no reply as whether it fixed the debug build or not. > > > Hope these pointers help. > > Regards, > Alexey > > On 25/03/2019 12:45, Thomas Stüfe wrote: > > Hi all, > > Following David's suggestion, I withdraw this bug and will open issues for > each area separately. > > Cheers, Thomas > > > > On Mon, Mar 25, 2019 at 1:44 PM Thomas Stüfe <[email protected]> > wrote: > >> Hi David, >> >> Updating vs2017 did not help :/ >> >> Cheers, Thomas >> >> On Mon, Mar 25, 2019 at 8:17 AM David Holmes <[email protected]> >> wrote: >> >>> Hi Thomas, >>> >>> On 25/03/2019 5:01 pm, Thomas Stüfe wrote: >>> > Hi David, >>> > >>> > (added net-dev, awt-dev, security-dev since part of these fixes are in >>> > their territory) >>> >>> May be better to split up the reviews, cross-posting that many groups >>> gets very messy given most people won't be subscribed to them all - >>> myself included. >>> >> >> >>> >>> > On Mon, Mar 25, 2019 at 1:34 AM David Holmes <[email protected] >>> > <mailto:[email protected]>> wrote: >>> > >>> > 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. :( >>> > >>> > >>> > Not sure what others do. I did a 32bit windows build, slowdebug, >>> warning >>> > enabled, and it failed with those 5+ issues. >>> > >>> > > 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. >>> > >>> > >>> > I use VS2017 CE. Not sure which version spcifically, but my compiler >>> is at >>> > >>> > Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26431 for x86 >>> >>> I think that would equate to an older version - 15.7 >>> >>> MSVC++ 14.14 _MSC_VER == 1914 (Visual Studio 2017 version 15.7) >>> >>> Any chance you can upgrade to latest version? (Especially in light of >>> the apparent compiler bug you cite below.) >>> >>> Thanks, >>> David >>> ----- >>> >>> > > 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" ?? >>> > >>> > >>> > I banged my head against this for an hour or so and I think this is a >>> > compiler bug. >>> > >>> > What I get is: >>> > >>> > warning C4838: conversion from '' to 'uint64_t' requires a narrowing >>> > conversion >>> > >>> > (Note the empty "from" string.) >>> > >>> > Here are my tries to provoke the error: >>> > >>> > VMLongConstantEntry iii[] = { { "hallo", >>> > markOopDesc::hash_mask_in_place }, {0,0}}; <<< this fails >>> > VMLongConstantEntry iii = { "hallo", markOopDesc::hash_mask_in_place >>> }; >>> > << but this succeeds >>> > uint64_t iii = markOopDesc::hash_mask_in_place; << this succeeds too >>> > >>> > I have no clue what this means. It is difficult to fix since the >>> > expression is hidden in such a macro pile. But I think either we go >>> with >>> > my change or we disable the warning for win32 for the whole section. >>> > >>> > > >>> > > 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. >>> > >>> > >>> > Okay, I will add them. >>> > >>> > Cheers, >>> > David >>> > ----- >>> > >>> > >>> > Thanks for reviewing, >>> > >>> > Thomas >>> > >>> > > Thanks, Thomas >>> > > >>> > >>> >> >
