RFR(M/L) : 8209611 : use C++ compiler for hotspot tests

[David Holmes]

On 21/08/2018 4:07 PM, Igor Ignatyev wrote:
>> On Aug 20, 2018, at 10:06 PM, David Holmes <david.holmes at 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.holmes at oracle.com <mailto:david.holmes at 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
>