RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition (original) (raw)

Kim Barrett kim.barrett at oracle.com
Sat Dec 19 00:41:49 UTC 2015

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 uintptrt, 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 *)(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

More information about the build-dev mailing list