<div dir="ltr">Hi all,<div><br></div><div>Following David's suggestion, I withdraw this bug and will open issues for each area separately.</div><div><br></div><div>Cheers, Thomas</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 25, 2019 at 1:44 PM Thomas Stüfe <<a href="mailto:thomas.stuefe@gmail.com">thomas.stuefe@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Hi David,</div><div><br></div><div>Updating vs2017 did not help :/<br></div><div><br></div><div>Cheers, Thomas</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 25, 2019 at 8:17 AM David Holmes <<a href="mailto:david.holmes@oracle.com" target="_blank">david.holmes@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Thomas,<br>
<br>
On 25/03/2019 5:01 pm, Thomas Stüfe wrote:<br>
> Hi David,<br>
> <br>
> (added net-dev, awt-dev, security-dev since part of these fixes are in <br>
> their territory)<br>
<br>
May be better to split up the reviews, cross-posting that many groups <br>
gets very messy given most people won't be subscribed to them all - <br>
myself included.<br></blockquote><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> On Mon, Mar 25, 2019 at 1:34 AM David Holmes <<a href="mailto:david.holmes@oracle.com" target="_blank">david.holmes@oracle.com</a> <br>
> <mailto:<a href="mailto:david.holmes@oracle.com" target="_blank">david.holmes@oracle.com</a>>> wrote:<br>
> <br>
>     Hi Thomas,<br>
> <br>
>     A few queries, comments and concerns ...<br>
> <br>
>     On 25/03/2019 6:58 am, Thomas Stüfe wrote:<br>
>      > Hi all,<br>
>      ><br>
>      > After a long time I tried to build a Windows 32bit VM (VS2017)<br>
>     and failed;<br>
> <br>
>     I'm somewhat surprised as I thought someone was actively doing Windows<br>
>     32-bit builds out there, plus there are shared code changes that should<br>
>     also have been caught by non-Windows 32-bit builds. :(<br>
> <br>
> <br>
> Not sure what others do. I did a 32bit windows build, slowdebug, warning <br>
> enabled, and it failed with those 5+ issues.<br>
> <br>
>      > multiple errors and warnings. Lets reverse the bitrot:<br>
>      ><br>
>      > cr:<br>
>      ><br>
>     <a href="http://cr.openjdk.java.net/~stuefe/webrevs/8221375--windows-32bit-build-(vs2017)-broken-in-many-places/webrev.00/webrev/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~stuefe/webrevs/8221375--windows-32bit-build-(vs2017)-broken-in-many-places/webrev.00/webrev/</a><br>
>      ><br>
>      > Issue: <a href="https://bugs.openjdk.java.net/browse/JDK-8221375" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8221375</a><br>
>      ><br>
>      > Most of the fixes are trivial: Calling convention mismatches (awt<br>
>     DTRACE<br>
>      > callbacks), printf specifiers etc.<br>
>      ><br>
>      > Had to supress a warning in os_windows_x86.cpp - I was surprised<br>
>     by this<br>
>      > since this did not look 32bit specifc, do we disable warnings on<br>
>     Windows<br>
>      > 64bit builds?<br>
> <br>
>     What version of VS2017? We use VS2017 15.9.6 and we don't disable<br>
>     warnings.<br>
> <br>
> <br>
> I use VS2017 CE. Not sure which version spcifically, but my compiler is at<br>
> <br>
> Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26431 for x86<br>
<br>
I think that would equate to an older version - 15.7<br>
<br>
MSVC++ 14.14 _MSC_VER == 1914 (Visual Studio 2017 version 15.7)<br>
<br>
Any chance you can upgrade to latest version? (Especially in light of <br>
the apparent compiler bug you cite below.)<br>
<br>
Thanks,<br>
David<br>
-----<br>
<br>
>      > The error I had in vmStructs.cpp was a bit weird: compiler<br>
>     complained about<br>
>      > an assignment of an enum value defined like this:<br>
>      ><br>
>      > hash_mask_in_place       = (address_word)hash_mask << hash_shift<br>
>      ><br>
>      > to an uint64_t variable, complaining about narrowing. I did not<br>
>     find out<br>
>      > what his problem was. In the end, I decided to add an explicit<br>
>     cast to<br>
>      > GENERATE_VM_LONG_CONSTANT_ENTRY(name) (see vmStructs.hpp).<br>
> <br>
>     Not at all sure that's the right fix. In markOop.hpp we see that value<br>
>     gets special treatment on Windows-x64:<br>
> <br>
> <br>
>     #ifndef _WIN64<br>
>                ,hash_mask               = right_n_bits(hash_bits),<br>
>                hash_mask_in_place       = (address_word)hash_mask <<<br>
>     hash_shift<br>
>     #endif<br>
>         };<br>
> <br>
>         // Alignment of JavaThread pointers encoded in object header<br>
>     required<br>
>     by biased locking<br>
>         enum { biased_lock_alignment    = 2 << (epoch_shift + epoch_bits)<br>
>         };<br>
> <br>
>     #ifdef _WIN64<br>
>           // These values are too big for Win64<br>
>           const static uintptr_t hash_mask = right_n_bits(hash_bits);<br>
>           const static uintptr_t hash_mask_in_place  =<br>
>                                   (address_word)hash_mask << hash_shift;<br>
>     #endif<br>
> <br>
>     perhaps something special is needed for Windows-x86. I'm unclear how<br>
>     the<br>
>     values can be "too big" ??<br>
> <br>
> <br>
> I banged my head against this for an hour or so and I think this is a <br>
> compiler bug.<br>
> <br>
> What I get is:<br>
> <br>
> warning C4838: conversion from '' to 'uint64_t' requires a narrowing <br>
> conversion<br>
> <br>
> (Note the empty "from" string.)<br>
> <br>
> Here are my tries to provoke the error:<br>
> <br>
> VMLongConstantEntry iii[] = {  { "hallo", <br>
> markOopDesc::hash_mask_in_place }, {0,0}};  <<< this fails<br>
> VMLongConstantEntry iii = { "hallo", markOopDesc::hash_mask_in_place };  <br>
>   << but this succeeds<br>
> uint64_t iii = markOopDesc::hash_mask_in_place;   << this succeeds  too<br>
> <br>
> I have no clue what this means. It is difficult to fix since the <br>
> expression is hidden in such a macro pile. But I think either we go with <br>
> my change or we disable the warning for win32 for the whole section.<br>
> <br>
>      ><br>
>      > With this patch we can build with warnings enabled on 32bit and 64bit<br>
>      > windows.<br>
>      ><br>
>      > Since patch fixes both hotspot and JDK parts, I'm sending it to<br>
>     hs-dev and<br>
>      > core-libs-dev.<br>
> <br>
>     Don't see anything that actually comes under core-libs-dev. Looks like<br>
>     one net-dev, one awt-dev and one security-dev. Sorry.<br>
> <br>
> <br>
> Okay, I will add them.<br>
> <br>
>     Cheers,<br>
>     David<br>
>     -----<br>
> <br>
> <br>
> Thanks for reviewing,<br>
> <br>
> Thomas<br>
> <br>
>      > Thanks, Thomas<br>
>      ><br>
> <br>
</blockquote></div></div>
</blockquote></div>