berenger-eu / tbfmm

Task-based fast multipole method, parallelized using OpenMP and StarPU. With StarPU it supports multiple GPUs (CUDA).
Other
9 stars 3 forks source link

MSVC does not support -openmp:llvm #8

Closed ArturSalamatin closed 1 year ago

ArturSalamatin commented 1 year ago

When I compile with MSVC Visual studio Community 2022 Release - amd64 under VS Code,

[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp( 72,9): error C7660: 'task': requires '-openmp:llvm -openmp:experimental' command line option(s) [.\tbfmm\build\testRotationKernel.vcxproj]
[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp(109,9): error C7660:......
[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp(172,9): error C7660:......
[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp(210,9): error C7660:......
[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp(323,9): error C7660:......
[build] .\tbfmm\src\algorithms/openmp/tbfopenmpalgorithm.hpp(260,9): error C7660:......

These errors reveal when the testRotationKernel target is built only. However, this should be a library-wide issue.

Can we do anything with this?

ArturSalamatin commented 1 year ago

Finally, I was able to compile a sequential version using MSVC. For this I have commented this CmakeLists.txt code

#[[
find_package(OpenMP)
if (OPENMP_FOUND)
    message(STATUS "OpenMP Found") 
    if(NOT ${TBFMM_ENABLE_OPENMP})
        message(STATUS "OpenMP Disabled")   
    else() 
        set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
        set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
        set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
        list(APPEND TBF_USE_LIST OPENMP)
    endif()
endif()

list(APPEND TBF_USE_LIST_AVAILABLE OPENMP)
]]

In a sequential version the line Line 148 algorithm->execute(tree); of testRotationKernel.cpp was executed successfully (while with OpenMP on at g++ the Segmentation Fault occurred here, issue #7 ).

But an error _STL_VERIFY(_Pos < _Size, "array subscript out of range"); is thrown at Line 230: tree.rebuild(); of testRotationKernel.cpp

ArturSalamatin commented 1 year ago

This article looks relevant to this issue https://devblogs.microsoft.com/cppblog/openmp-task-support-for-c-in-visual-studio/

I am going to study it soon.

berenger-eu commented 1 year ago

So it seems you need to pass -openmp:llvm -openmp:experimental as stated by your compiler. This is weird because cmake detect openmp (this is why it enters the if statement you did comment), but then it cannot compile with openmp... Anyway, what could be done would be to replace the commented section with:

if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
    message(STATUS "MSVC detected")
    if(NOT ${TBFMM_ENABLE_OPENMP})
        message(STATUS "OpenMP Disabled")   
    else()
        message(STATUS "OpenMP enabled with  -openmp:llvm -openmp:experimental")   
        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -openmp:llvm -openmp:experimental")
    endif()
else()   
    find_package(OpenMP)
    if (OPENMP_FOUND)
        message(STATUS "OpenMP Found") 
        if(NOT ${TBFMM_ENABLE_OPENMP})
            message(STATUS "OpenMP Disabled")   
        else() 
            set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
            set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
            set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
            list(APPEND TBF_USE_LIST OPENMP)
        endif()
    endif()
endif()
list(APPEND TBF_USE_LIST_AVAILABLE OPENMP)

Could you give it a try? It will not fix the segfault, but at least it should compile.

ArturSalamatin commented 1 year ago

Yes, I have tried this. And I was able to compile the code using MSVC compiler. A pull request is created.

9