<AWT Dev> RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Kim Barrett]

On Dec 16, 2015, at 8:50 AM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
> 
> There is an interest from the community to build OpenJDK using Visual Studio 2015 Community edition. 
> 
> This patch is provided by Timo Kinnunen <timo.kinnunen at gmail.com>. I am sponsoring this patch.
> 
> The changes to the source code files are mostly casts to uintptr_t, but there are some other changes as well.
> 
> I'm not quite sure who's the owner of all these files. If I'm missing some group, please help me and forward the mail to them.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
> WebRev: http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
> 
> /Magnus

I only looked at hotspot files.

Breaking this up into smaller and more focused chunks would be nice.
Large changes that do 17 different things are hard to deal with.

I think the various casts to uintptr_t (commented with "What???"
below) should be done differently.  They are addressing problems of
differences between pointer size and various integer sizes, and
conversions between them.  I think in some cases there is a poorly
chosen surrounding type involved.  For the 0xdeadbeef cases, there
probably ought to be a centralized helper for that sort of thing,
since doing it properly on across different platforms might be tricky,
and that trickiness ought to be in one place.

There are a couple of "TBD"s below that I need to spend more time on.

------------------------------------------------------------------------------
Sprinkling lots of files with the following seems like a bad idea.

#if _MSC_VER >= 1900
#pragma warning( disable : 4091 ) // typedef ignored on left when no variable is declared
#endif

Especially since these warnings may indicate actual bugs.
https://msdn.microsoft.com/en-us/library/eehkcz60.aspx

------------------------------------------------------------------------------ 
hotspot/src/share/vm/utilities/globalDefinitions.hpp
1062 #define       badAddress        ((address)(intptr_t)::badAddressVal)

Change the type of badAddressVal to intptr_t instead.

------------------------------------------------------------------------------ 
hotspot/src/share/vm/runtime/os.cpp
 127 #elif defined(_WIN32) && _MSC_VER > 1800
 128   const time_t zone = _timezone;

Why does (recent) Windows need special handling here?

------------------------------------------------------------------------------ 
hotspot/src/share/vm/utilities/globalDefinitions_visCPP.hpp
 198 #if _MSC_VER <= 1800

Shouldn't that be "<" the version that introduced the missing
function?

------------------------------------------------------------------------------ 
hotspot/src/share/vm/prims/whitebox.cpp
1286   return (jlong)(uintptr_t)ikh->constants();

What???

------------------------------------------------------------------------------
hotspot/src/share/vm/opto/type.cpp
  53   { Bad,             T_ILLEGAL,    "bad",           false, (int)Node::NotAMachineReg, relocInfo::none          },  // Bad
  61   { Bad,             T_ILLEGAL,    "tuple:",        false, (int)Node::NotAMachineReg, relocInfo::none          },  // Tuple
  62   { Bad,             T_ARRAY,      "array:",        false, (int)Node::NotAMachineReg, relocInfo::none          },  // Array

These are narrowing conversions in brace initializers, which are
forbidden by C++11.  The type of the member is int, while the
initialization expressions are non-literal uint.

In my opinion, it would be better to figure out how to fix the type
mismatch than to sprinkle with casts.

A way to look for these that doesn't require VS2015 would be to use
g++ -Wnarrowing.

------------------------------------------------------------------------------ 
hotspot/src/share/vm/opto/split_if.cpp
 258   Node *prior_n = (Node*)(uintptr_t)0xdeadbeef;
 305   prior_n = (Node*)(uintptr_t)0xdeadbeef;  // Reset IDOM walk

What???

------------------------------------------------------------------------------ 
hotspot/src/share/vm/opto/gcm.cpp
1448   _node_latency = (GrowableArray<uint> *)(uintptr_t)0xdeadbeef;

What???

------------------------------------------------------------------------------ 
hotspot/src/share/vm/opto/compile.cpp
2398   _cfg     = (PhaseCFG*)(uintptr_t)0xdeadbeef;
2399   _regalloc = (PhaseChaitin*)(uintptr_t)0xdeadbeef;

What???

------------------------------------------------------------------------------
hotspot/src/share/vm/opto/buildOopMap.cpp
 618     Block *pred = (Block*)(uintptr_t)0xdeadbeef;

What???

------------------------------------------------------------------------------ 
hotspot/src/share/vm/oops/typeArrayOop.hpp
 132     *long_at_addr(which) = (jlong)contents;

Yes!  Fortunately, it looks like metadata_at_put is never called.

------------------------------------------------------------------------------ 
hotspot/src/share/vm/memory/allocation.hpp
  38 #if defined _WINDOWS && _MSC_VER >= 1900
  39  // 'noexcept' used with no exception handling mode specified; termination on exception is not guaranteed. Specify /EHsc
  40 #pragma warning( disable : 4577 )
  41 #endif

Another C++11 change.  Dynamic exception specifications
(e.g. "throw(TYPELISTopt)" were deprecated by C++11.  I think MSVC++
has always implemented "throw()" as "throwing an exception invokes
undefined behavior".  C++11 introduced "noexcept".

I think it would be better to turn this off in the build system,
rather than with with this conditional #pragma.

Alternatively, perhaps replace existing uses of "throw()" with a
VM_NOEXCEPT macro (or some other name) which expands into "throw()" or
"noexcept" depending on the compiler and options.

------------------------------------------------------------------------------ 
hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp
-- many, many lines --

Ouch!  I'm not looking at this in detail...

------------------------------------------------------------------------------ 
hotspot/src/share/vm/adlc/arena.hpp
  74   // Usual (non-placement) deallocation function to allow placement delete use size_t
  75   // See 3.7.4.2 [basic.stc.dynamic.deallocation] paragraph 2.
  76   void  operator delete(void* p);

and associated code in the .cpp file.

--- TBD

------------------------------------------------------------------------------ 
hotspot/agent/src/share/native/sadis.c
  96       return (int)strlen(buf);

--- TBD

------------------------------------------------------------------------------