-
Notifications
You must be signed in to change notification settings - Fork 563
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
Corrected functions to compile with VS2022 #13319
Corrected functions to compile with VS2022 #13319
Conversation
…_20240712_180111 Automatically Merged using Trilinos Master Merge AutoTester PR Title: b'Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20240712_180111 branch to master' PR Author: trilinos-autotester
…_20240719_180002 Automatically Merged using Trilinos Master Merge AutoTester PR Title: b'Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20240719_180002 branch to master' PR Author: trilinos-autotester
…_20240726_180005 Automatically Merged using Trilinos Master Merge AutoTester PR Title: b'Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20240726_180005 branch to master' PR Author: trilinos-autotester
@ViNN280801 Please open a PR with these changes directly against https://github.com/kokkos/kokkos. For the Kokkos-Kernels specific issue, please report it here: https://github.com/kokkos/kokkos-kernels. We do not test Trilinos on Windows, but since you are building there: would you mind giving more details on the other issues you are seeing? In particular, what syntax errors are there? |
Okay, I'll do that. As for syntax errors, I'm sure it's because of some compiler settings. Because I successfully compiled the entire Trilinos on Fedora 34 and Debian 12, but now I needed to do it on Windows and it turned out to be problematic. Examples of the "syntax errors": false positives of some kind. |
@ViNN280801 Could you try replacing @iyamazaki FYI. |
just the same |
@ViNN280801 I think you might have replaced Trilinos/packages/kokkos/core/src/Kokkos_Macros.hpp Lines 195 to 199 in 19640f3
|
@cgcgcg, yes, this solved some of the problems, but I'm still in the process of building for Windows, as soon as it works, I'll provide instructions on how I managed to do it. |
@ViNN280801 Thanks! |
The Kokkos changes would need submission of corresponding PR to https://github.com/kokkos/kokkos , and acceptance to the repo before acceptance+merging to Trilinos; adding @dalg24 @crtrott @masterleinad for tracking |
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
…_20240806_153426 Automatically Merged using Trilinos Master Merge AutoTester PR Title: b'Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20240806_153426 branch to master' PR Author: trilinos-autotester
So, I collected some information that I've done to compile almost all the packages (Amesos2, Belos, Intrepid2, Kokkos, KokkosKernels, NOX, ROL, RTOp, Sacado, Shards, Stokhos, Stratimikos, Tacho, Teuchos, Tempus, Thyra, Tpetra, MiniTensor, Adelus, SEACAS, ShyLU, Galeri). I had a lot of compilation issues with C++ std version (changed for all the projects to C++20), problems with FMT support of the utf-8 (added flag VS22 printed to me unresolved links and some issues like "Cannot find After the build I have the setup file What about the code, I changed some files, and I wrote these files with changes: Fixing fmt issue constant expr in e.txt I compile the Trilinos with MSYS2 using the following command (added some flags like utf-8 and c++20 std): cmake -G "Visual Studio 17 2022" \
-DCMAKE_BUILD_TYPE=Release \
-DTrilinos_ENABLE_TESTS=OFF \
-DTrilinos_ENABLE_ALL_PACKAGES=ON \
-DTPL_ENABLE_MPI=ON \
-DTrilinos_ENABLE_Epetra=OFF \
-DTrilinos_ENABLE_EpetraExt=OFF \
-DTrilinos_ENABLE_Tpetra=ON \
-DCMAKE_C_COMPILER=gcc \
-DCMAKE_CXX_COMPILER=g++ \
-DMPI_EXEC="C:/Program Files/Microsoft MPI/Bin/mpiexec.exe" \
-DMPI_C_COMPILER="C:/msys64/mingw64/bin/mpicc.exe" \
-DMPI_CXX_COMPILER="C:/msys64/mingw64/bin/mpicxx.exe" \
-DCMAKE_C_FLAGS="/std:c17" \
-DCMAKE_CXX_FLAGS="/std:c++20 /utf-8" \
-DCMAKE_INCLUDE_PATH="$ENV{INCLUDE}" \
-DCMAKE_LIBRARY_PATH="\"C:/Program Files/fmt/lib\";\"C:/Program Files (x86)/Microsoft SDKs/MPI/Lib/x64\"" \
-DCMAKE_EXE_LINKER_FLAGS="/LIBPATH:\"C:/Program Files/fmt/lib\" /LIBPATH:\"C:/Program Files (x86)/Microsoft SDKs/MPI/Lib/x64\"" \
.. But before I did this: pacman -Syu pacman -S mingw-w64-x86_64-toolchain mingw-w64-x86_64-cmake mingw-w64-x86_64-cmake mingw-w64-x86_64-msmpi mingw-w64-x86_64-openblas mingw-w64-x86_64-blas mingw-w64-x86_64-netcdf mingw-w64-x86_64-lapack Windows have a constraint with length of the files (260 if I correctly understand), so to shorten the paths, at first I turned on long paths using group policy (gpedit.msc), and moved to the C:/ dir:
Keyword restrict (__restrict)Last point is the restrict keyword that ruins some packages:
As a result result I have Trilinos here: I checked my installation on my project, and everything seems to be fine: cmake -G "Visual Studio 17 2022" `
-DMPIEXEC_EXECUTABLE:FILEPATH="C:/Program Files/Microsoft MPI/Bin/mpiexec.exe" `
-DMPI_C_HEADER_DIR:PATH="C:/Program Files (x86)/Microsoft SDKs/MPI/Include" `
-DMPI_CXX_HEADER_DIR:PATH="C:/Program Files (x86)/Microsoft SDKs/MPI/Include" `
-DMPI_C_LIB_NAMES:STRING="msmpi" `
-DMPI_CXX_LIB_NAMES:STRING="msmpi" `
-DMPI_msmpi_LIBRARY:FILEPATH="C:/Program Files (x86)/Microsoft SDKs/MPI/Lib/x64/msmpi.lib" `
-DMPI_C_LIBRARIES:FILEPATH="C:/Program Files (x86)/Microsoft SDKs/MPI/Lib/x64/msmpi.lib" `
-DMPI_CXX_LIBRARIES:FILEPATH="C:/Program Files (x86)/Microsoft SDKs/MPI/Lib/x64/msmpi.lib" `
-DMPI_C_COMPILER="cl.exe" `
-DMPI_CXX_COMPILER="cl.exe" `
.. VS22 sees includes: |
And I have a user-friendly suggestion - it'll be very convenient if Trilinos has precompiled files for different architechtures, like mpi, tbb, gmsh, and others have. Because:
|
This PR should be set to merge against |
…_20240809_175814 Automatically Merged using Trilinos Master Merge AutoTester PR Title: b'Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20240809_175814 branch to master' PR Author: trilinos-autotester
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
…'__restrict' keyword instead of Linux '__restrict__'. So, all occurences with '__restrict__' replaced with 'KOKKOS_RESTRICT' macro to provide correct compilation under MSVC
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
1 similar comment
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
Thank you for the changes, @ViNN280801!! I am locally building just to be sure. So, the main changes are to replace |
@iyamazaki, yes, just replace all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @ViNN280801! I could also build and run example on V100.
Presumably you want to target the develop branch though? |
Sorry.. I thought it has been already changed, can you switch to merge into |
@iyamazaki, done! |
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; this inspection will remain valid until a new commit to source branch is performed. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
Using Repos:
Pull Request Author: ViNN280801 |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: PR_gcc-openmpi-openmp
Jenkins Parameters
Build InformationTest Name: PR_gcc
Jenkins Parameters
Build InformationTest Name: PR_gcc-openmpi_debug
Jenkins Parameters
Build InformationTest Name: PR_clang
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Build InformationTest Name: PR_cuda
Jenkins Parameters
Build InformationTest Name: PR_intel
Jenkins Parameters
Build InformationTest Name: PR_cuda-uvm
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ iyamazaki ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 13319: IS A SUCCESS - Pull Request successfully merged |
@trilinos/kokkos
Motivation
This change addresses a compilation issue under MSVC in Visual Studio 2022. The compiler reports a C2535 error indicating that two overloaded functions are identical, despite having different
enable_if
conditions. Adding additional conditions to theenable_if
clauses ensures that the MSVC compiler can distinguish between the two overloaded functions.Testing
The changes have been tested locally to ensure that they resolve the compilation issue without affecting the functionality of the code. The functions are covered by existing unit tests in the Kokkos package.
Additional Information
Request for Improved Windows Support
During the process of compiling Trilinos on Windows, I encountered numerous issues related to the build system and compatibility with MSVC in Visual Studio 2022. These challenges included but were not limited to:
Error C1083 Cannot open source file: 'C:\Users\vladislavsemykin\Downloads\Trilinos\Trilinos\build\packages\kokkos-kernels\sparse\eti\generated_specializations_cpp\gauss_seidel_symbolic\Sparse_gauss_seidel_symbolic_eti_DOUBLE_ORDINAL_INT_OFFSET_SIZE_T_LAYOUTLEFT_EXECSPACE_SERIAL_MEMSPACE_HOSTSPACE.cpp': No such file or directory kokkoskernels C:\Users\vladislavsemykin\Downloads\Trilinos\Trilinos\build\packages\kokkos-kernels\sparse\eti\generated_specializations_cpp\gauss_seidel_symbolic\Sparse_gauss_seidel_symbolic_eti_DOUBLE_ORDINAL_INT_OFFSET_SIZE_T_LAYOUTLEFT_EXECSPACE_SERIAL_MEMSPACE_HOSTSPACE.cpp 1
). I have this file, but still getting an error, so, I guess that naming is too long.
Given the increasing use of Windows as a development platform, I kindly request the Trilinos development team to consider making the build process more accessible and robust for Windows users. Enhancements such as detailed Windows-specific build instructions, automated scripts to handle common issues, and pre-configured environment setups would greatly benefit the community.
Thank you for considering this request, and please let me know if there is anything I can do to assist with improving the Windows build process.