On Tue, 5 Dec 2023 11:19:00 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> Hi Marcus (@magicus), please see the updated code which added guards to 
>> check for GCC version >= 7.5 in 
>> `src/java.base/linux/native/libsimdsort/{avx2-linux-qsort.cpp, 
>> avx512-linux-qsort.cpp}`. GCC >= 7.5 is needed to compile libsimdsort using 
>> C++17 features. Made sure that OpenJDK builds without errors using both GCC 
>> 7.5 and GCC 6.4.
>
> That sounds weird. You can't check for if compiler options should be enabled 
> or not inside source code files.
> 
> Are you saying that when compiling with GCC 6, it will just silently ignore 
> `-std=c++17`? I'd have assumed that it printed a warning or error about an 
> unknown or invalid option, if C++17 is not supported.

Hi Magnus (@magicus),
 
> Are you saying that when compiling with GCC 6, it will just silently ignore 
> `-std=c++17`? I'd have assumed that it printed a warning or error about an 
> unknown or invalid option, if C++17 is not supported.

The GCC complier for versions 6 (and even 5) silently ignores the flag 
`-std=c++17`. It does not print any warning or error. I tested it with a toy 
C++ program and also by building OpenJDK using GCC 6. 

> You can't check for if compiler options should be enabled or not inside 
> source code files.

 what I meant was, there are #ifdef guards using predefined macros in the C++ 
source code to check for GCC version and make the simdsort code available for 
compilation or not based on the GCC version


// src/java.base/linux/native/libsimdsort/simdsort-support.hpp
#if defined(_LP64) && (defined(__GNUC__) && ((__GNUC__ > 7) || ((__GNUC__ == 7) 
&& (__GNUC_MINOR__ >= 5))))
        #define __SIMDSORT_SUPPORTED_LINUX
#endif



//src/java.base/linux/native/libsimdsort/avx2-linux-qsort.cpp
#include "simdsort-support.hpp"
#ifdef __SIMDSORT_SUPPORTED_LINUX
            <simdsort functions>
#endif

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16534#discussion_r1416037340

Reply via email to