On Dec 16, 2015, at 8:50 AM, Magnus Ihse Bursie <magnus.ihse.bur...@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.kinnu...@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 ------------------------------------------------------------------------------