Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Adding support for Intel LLVM compiler (aka ICX, aka DPC++) #12880

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

loumalouomega
Copy link
Member

@loumalouomega loumalouomega commented Nov 27, 2024

📝 Description

This PR involves changes to the kratos_export_api.h file:

  1. Header has been cleaned up.

  2. The header guard (#ifndef KRATOS_EXPORT_API_H) was replaced with a #pragma once directive, which is a modern and more efficient way to prevent multiple inclusions of the header.

  3. Conditional declarations of explicit template instances (KRATOS_API_EXTERN) were updatedto accommodate Intel LLVM (aka ICX, aka DPC++) compilers on Windows.

For now compilation only works with Ninja, settings must be as following:

@echo off
rem Please do not modify this script

rem For any question please contact with us in:
rem  - https://github.com/KratosMultiphysics/Kratos

rem Optional parameters:
rem You can find a list will all the compiation options in INSTALL.md or here:
rem  - https://github.com/KratosMultiphysics/Kratos/wiki/Compilation-options

rem Set variables
if not defined KRATOS_SOURCE set KRATOS_SOURCE=%~dp0..
if not defined KRATOS_BUILD set KRATOS_BUILD=%KRATOS_SOURCE%/build

rem oneAPI call and set compiler
call "C:\Program Files (x86)\Intel\oneAPI\setvars.bat"
set CC=icx.exe
set CXX=icx.exe
@REM Only Ninja compiler is supported with Intel LLVM
set CMAKE_GENERATOR=Ninja


rem Warning: In windows this option only works if you run through a terminal with admin privileges
rem set KRATOS_INSTALL_PYTHON_USING_LINKS=ON

rem Set basic configuration
if not defined KRATOS_BUILD_TYPE set KRATOS_BUILD_TYPE=Release
if not defined BOOST_ROOT set BOOST_ROOT=C:\CompiledLibs\boost_1_67_0
if not defined PYTHON_EXECUTABLE set PYTHON_EXECUTABLE=C:\Windows\py.exe
if not defined NUMBER_OF_COMPILATION_CORES set NUMBER_OF_COMPILATION_CORES=%NUMBER_OF_PROCESSORS%

rem Set applications to compile
set KRATOS_APP_DIR=applications
set KRATOS_APPLICATIONS=
CALL :add_app %KRATOS_APP_DIR%\LinearSolversApplication;
CALL :add_app %KRATOS_APP_DIR%\StructuralMechanicsApplication;
CALL :add_app %KRATOS_APP_DIR%\FluidDynamicsApplication;

rem Clean
del /F /Q "%KRATOS_BUILD%\%KRATOS_BUILD_TYPE%\cmake_install.cmake"
del /F /Q "%KRATOS_BUILD%\%KRATOS_BUILD_TYPE%\CMakeCache.txt"
del /F /Q "%KRATOS_BUILD%\%KRATOS_BUILD_TYPE%\CMakeFiles"

rem Enable this if your build is slow and you have a multi-core machine
rem set KRATOS_PARALLEL_BUILD_FLAG=/MP4

rem Configure
@echo on
cmake                                              ^
-G "%CMAKE_GENERATOR%"                             ^
-H"%KRATOS_SOURCE%"                                ^
-B"%KRATOS_BUILD%\%KRATOS_BUILD_TYPE%"             ^
-DUSE_EIGEN_MKL=OFF                                ^
-DCMAKE_CXX_FLAGS=" %KRATOS_PARALLEL_BUILD_FLAG% " ^
-DKRATOS_GENERATE_PYTHON_STUBS=ON

rem Build
cmake --build "%KRATOS_BUILD%/%KRATOS_BUILD_TYPE%" --target install -- -j%NUMBER_OF_COMPILATION_CORES%
goto:eof

rem Function to add apps
:add_app
set KRATOS_APPLICATIONS=%KRATOS_APPLICATIONS%%1;
goto:eof

Thanks @roigcarlo

🆕 Changelog

@loumalouomega
Copy link
Member Author

We may think to add the compìler to the CI?

@loumalouomega
Copy link
Member Author

We may think to add the compìler to the CI?

Maybe replacing the just core compilation?

@loumalouomega loumalouomega marked this pull request as draft November 27, 2024 20:07
@roigcarlo
Copy link
Member

Just as track, currently we have an hard blocker in the linking stage as we are exceeding the number of exposed symbols allowed by the linker.

Should be possible to overcome with -bigobj or /bigobj but the icx-2025 seems to ignore those.

@loumalouomega
Copy link
Member Author

Just as track, currently we have an hard blocker in the linking stage as we are exceeding the number of exposed symbols allowed by the linker.

Should be possible to overcome with -bigobj or /bigobj but the icx-2025 seems to ignore those.

Looks like the abbys is looking us...

@matekelemen
Copy link
Contributor

Maybe it's finally time to hide symbols by default on compilers other than MSVC too.

@loumalouomega
Copy link
Member Author

Maybe it's finally time to hide symbols by default on compilers other than MSVC too.

Can you please elaborate?

@matekelemen
Copy link
Contributor

matekelemen commented Dec 11, 2024

Can you please elaborate?

If you have a compiled lib and want to use some stuff (functions, classes, etc) from it, that stuff must be exposed by the lib. "Stuff" in a compiled binary are identified by "symbols", and may or may not be exposed (i.e. visible to outside users). Think of functions internal to the library that aren't meant to be invoked by users.

The choice of whether or not to export symbols is the developer's decision but is often forgotten about because of default and global compiler settings. GCC and Clang (and by extension I imagine dpcpp too) export everything by default, but MSVC does the opposite: nothing is exported by default and you have to explicitly mark whatever you want accessible in your binary (we use KRATOS_API for that).

This results in the shitty situations I find myself too often in: I compile kratos locally (using gcc or clang) and everything works perfectly. But then windows fails in the CI with linker errors because I forgot to export my newly added stuff.

This is one of the rare occasions when I think that MSVC has the more sensible defaults. I think we should change the export policy on all our compilers not to export symbols by default. We already require everything to run on MSVC so theoretically it's a dead simple change (ideally, no C++ code would have to be changed).

Exporting everything bloats our binaries and has some performance impact too as a result. Vomiting everything on our interface is also pretty ugly in my opinion.

@loumalouomega
Copy link
Member Author

Can you please elaborate?

If you have a compiled lib and want to use some stuff (functions, classes, etc) from it, that stuff must be exposed by the lib. "Stuff" in a compiled binary are identified by "symbols", and may or may not be exposed (i.e. visible to outside users). Think of functions internal to the library that aren't meant to be invoked by users.

The choice of whether or not to export symbols is the developer's decision but is often forgotten about because of default and global compiler settings. GCC and Clang (and by extension I imagine dpcpp too) export everything by default, but MSVC does the opposite: nothing is exported by default and you have to explicitly mark whatever you want accessible in your binary (we use KRATOS_API for that).

This results in the shitty situations I find myself too often in: I compile kratos locally (using gcc or clang) and everything works perfectly. But then windows fails in the CI with linker errors because I forgot to export my newly added stuff.

This is one of the rare occasions when I think that MSVC has the more sensible defaults. I think we should change the export policy on all our compilers not to export symbols by default. We already require everything to run on MSVC so theoretically it's a dead simple change (ideally, no C++ code would have to be changed).

Exporting everything bloats our binaries and has some performance impact too as a result. Vomiting everything on our interface is also pretty ugly in my opinion.

I found this: https://gcc.gnu.org/wiki/Visibility, do you have an idea to do it in a generic way with minimal code change?, I guess that refactoring the KRATOS_API in POSIX cases

@loumalouomega
Copy link
Member Author

Can you please elaborate?

If you have a compiled lib and want to use some stuff (functions, classes, etc) from it, that stuff must be exposed by the lib. "Stuff" in a compiled binary are identified by "symbols", and may or may not be exposed (i.e. visible to outside users). Think of functions internal to the library that aren't meant to be invoked by users.
The choice of whether or not to export symbols is the developer's decision but is often forgotten about because of default and global compiler settings. GCC and Clang (and by extension I imagine dpcpp too) export everything by default, but MSVC does the opposite: nothing is exported by default and you have to explicitly mark whatever you want accessible in your binary (we use KRATOS_API for that).
This results in the shitty situations I find myself too often in: I compile kratos locally (using gcc or clang) and everything works perfectly. But then windows fails in the CI with linker errors because I forgot to export my newly added stuff.
This is one of the rare occasions when I think that MSVC has the more sensible defaults. I think we should change the export policy on all our compilers not to export symbols by default. We already require everything to run on MSVC so theoretically it's a dead simple change (ideally, no C++ code would have to be changed).
Exporting everything bloats our binaries and has some performance impact too as a result. Vomiting everything on our interface is also pretty ugly in my opinion.

I found this: gcc.gnu.org/wiki/Visibility, do you have an idea to do it in a generic way with minimal code change?, I guess that refactoring the KRATOS_API in POSIX cases

Gemini suggested this:

Hiding Symbols in GCC: A Comprehensive Guide

Hiding symbols in GCC involves controlling the visibility of symbols, preventing them from being exported and accessible to other modules. This can be achieved using various compiler and linker flags.

Understanding Symbol Visibility

Before diving into the techniques, it's essential to understand the two primary visibility levels:

  1. Default Visibility: Symbols are visible to other modules and can be linked.
  2. Hidden Visibility: Symbols are only visible within the current compilation unit and cannot be linked to from other modules.

Techniques to Hide Symbols

1. Compiler Flag: -fvisibility=hidden

This flag sets the default visibility of all symbols to hidden. To export specific symbols, you'll need to explicitly mark them with the **attribute**((visibility("default"))) attribute:

__attribute__((visibility("default"))) void my_visible_function() {
    // ...
}

void my_hidden_function() {
    // ...
}

Compile with:

gcc -fvisibility=hidden -shared -o my_library.so my_file.c

2. Linker Flag: -Wl,--exclude-libs=ALL

This flag instructs the linker to hide all symbols from static libraries linked into your shared library. It's particularly useful when you want to prevent symbols from a static library from being exported by your shared library:

gcc -shared -o my_library.so -Wl,--exclude-libs=ALL my_file.c -lstatic_library

3. Compiler Attribute: **attribute**((visibility("hidden")))

You can directly apply this attribute to individual symbols to make them hidden:

void my_hidden_function() __attribute__((visibility("hidden")));

Additional Considerations

  • Static Libraries: Hiding symbols in static libraries is less straightforward. You can use the static keyword to declare functions and variables with internal linkage, but this limits their visibility to the current compilation unit.
  • Stripping Symbols: While not strictly hiding, you can use the strip utility to remove unnecessary symbols from object files and executables, reducing their size and potential security risks.
  • Security Implications: Be cautious when hiding symbols. While it can protect your intellectual property, it can also make debugging and troubleshooting more difficult.

By understanding these techniques and their implications, you can effectively control symbol visibility in your GCC projects, balancing security, performance, and maintainability.

@roigcarlo
Copy link
Member

My two cents. We tried in the past to had the -fvisibility=hidden enabled to behave as win but never succeeded, because either gcc or clang (don'r temember which one) or one of its linkers ignores the flag.

From code prespective, since the macro exists, and there is already a place for linux, if we want to implement this it should be relatively painless, but I never managed to finish it correctly.

@loumalouomega
Copy link
Member Author

My two cents. We tried in the past to had the -fvisibility=hidden enabled to behave as win but never succeeded, because either gcc or clang (don'r temember which one) or one of its linkers ignores the flag.

From code prespective, since the macro exists, and there is already a place for linux, if we want to implement this it should be relatively painless, but I never managed to finish it correctly.

Probaly Clang was the issue, maybe the flag is different

@matekelemen
Copy link
Contributor

We tried in the past to had the -fvisibility=hidden enabled to behave as win but never succeeded, because either gcc or clang (don'r temember which one) or one of its linkers ignores the flag.

Have you tried setting VISIBILITY_INLINES_HIDDEN to hidden in CMake?

I'd prefer letting CMake do its thing if possible.

@roigcarlo
Copy link
Member

Nope, we didn't tried that. If you manage to make it work we can merge it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants