On 21/08/2018 4:07 PM, Igor Ignatyev wrote:
On Aug 20, 2018, at 10:06 PM, David Holmes <david.hol...@oracle.com> wrote:
Hi Igor,
On 21/08/2018 1:58 PM, Igor Ignatev wrote:
Hi David,
It is necessary for vmTestbase tests (if we of course want to clean them up) as
they are coupled, and moving only part of them will require (non trivial)
updates in the rest of code (or at least in the shared part) to properly serve
both compilers.
Can you elaborate on the problem please as I don't see why the vmTestbase tests
are special here.
I would expect a .c file to be compiled as C and a .cpp to be compiled as C++.
currently our build system supports only 1-1 relation b/w tests lib/exec and
source file; most of native libraries in vmTestbase have more than one source
file. to work that around, we introduced a .c file for each library and theses
.c files include all other required .c files.
yes, a .c file will be compiled as C and a .cpp file as C++, but as we have .c
files which include .c files, moving only part of .c files, we will end up w/
.c files being included into both .c and .cpp files and thus compiled as both C
and C++. let's take
test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS301/hs301t004, if
we want its native part to be compiled as C++, we rename libhs301t004.c to
libhs301t004.cpp, but libhs301t004.c includes 11 other files, there only one is
a test specific fill and all the other are used by other tests and included by
their .c files. so we have 10 files which now will be compiled by both C and
C++ compilers.
Okay, but why do we need to change any of these existing tests to be
compiled as C++?
David
------
It absolutely not necessary for other tests, but I’d prefer to have some level
of unification in the test base. That being said, I agree I might went too far
and moved all the tests which might or might not compromise their purpose. I
personally don’t think we should relay on our jtreg testbase to verify if C
linked libraries can be used with hotspot. it must be verified by JCK which
should be compiled/linked with carefully chosen compilers/linkers and their
flags.
Sure but JCK comes into play much later and I'd rather spot issues during
developer testing than promotion testing.
agree, but if we want to verify how C/C++ linked libraries work w/ hotspot, it
should be done by the tests specifically written for that purpose rather that
relaying on indirect coverage from other tests, and I guess it hasn't been done
as we always relied on JCK to test that.
It has been discussed (not widely enough and I accept that) in 8209547 and the
related email thread b/w JC(cc'ed) and myself.
as I said, I might went a way too far, so I'll revert changes in the
non-vmTestbase tests and made appropriate changes in makefiles. what do you
think?
I think I need to see what you mean exactly :)
sure, it will take some time for me to do that, hopefully will upload new
webrevs tomorrow morning PT. but the basic idea is to leave files in
test/hotspot/jtreg/compiler, runtime, gc, native_sanity, serviceability,
testlibrary as .c files, exactly as they were before, and restore corresponding
filenames in make/test/JtregNativeHotspot.gmk.
Cheers,
-- Igor
Thanks,
David
Thanks,
— Igor
On Aug 20, 2018, at 6:43 PM, David Holmes <david.hol...@oracle.com
<mailto:david.hol...@oracle.com>> wrote:
Hi Igor,
On 21/08/2018 8:59 AM, Igor Ignatyev wrote:
http://cr.openjdk.java.net/~iignatyev//8209611/webrev.01/index.html
<http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.01/index.html>
11160 lines changed: 879 ins; 61 del; 10220 mod;
Hi all,
could you please review the patch which moves all hotspot native test code to
C++? this will guarantee that we always use C++ compilers for them (as an
opposite to either C or C++ compiler depending on configuration), as a result
we will be able to get rid of JNI_ENV_ARG[1] macros, perform other clean ups
and improve overall quality of the test code.
Sorry but I don't see why this is necessary. If people want to be able to write
C++ tests then we should enable that if not currently enabled, but I don't see
why everything should be forced to C++. What if we did something that broke the
C linkage and we didn't detect it because we only ever tested C++?
Was the motivation previously discussed somewhere?
Thanks,
David
the patch consists of two parts:
- automatic: renaming .c files to .cpp, updating #include, changing JNI/JVMTI
calls
- semi-manual: adding extern "C" , fixing a number of compiler warnings
(mostly types inconsistency), updating makefiles
JBS: https://bugs.openjdk.java.net/browse/JDK-8209611
webrevs:
- automatic: http://cr.openjdk.java.net/~iignatyev//8209611/webrev.00/index.html
<http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.00/index.html>
9394 lines changed: 0 ins; 0 del; 9394 mod;
- semi-manual: http://cr.openjdk.java.net/~iignatyev//8209611/webrev.0-1/index.html
<http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.0-1/index.html>
1899 lines changed: 879 ins; 61 del; 959 mod
- whole: http://cr.openjdk.java.net/~iignatyev//8209611/webrev.01/index.html
<http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.01/index.html>
11160 lines changed: 879 ins; 61 del; 10220 mod;
testing: all hotspot tests + tier[1-3]
[1] https://bugs.openjdk.java.net/browse/JDK-8209547
Thanks,
-- Igor
