ParU fails with OpenMP from MSVC

[mmuetzel]

Describe the bug
When trying to build ParU with an MSVC toolchain, compilation fails with errors like the following:

  D:\a\SuiteSparse\SuiteSparse\ParU\Source\paru_analyze.cpp(1060): error C3001: 'taskloop': expected an OpenMP directive name
  D:\a\SuiteSparse\SuiteSparse\ParU\Source\paru_analyze.cpp(1068): error C3001: 'taskloop': expected an OpenMP directive name
  D:\a\SuiteSparse\SuiteSparse\ParU\Source\paru_analyze.cpp(1300): error C3001: 'taskloop': expected an OpenMP directive name
  D:\a\SuiteSparse\SuiteSparse\ParU\Source\paru_analyze.cpp(1314): error C3001: 'taskloop': expected an OpenMP directive name
  [3/84] Building CXX object CMakeFiles\paru.dir\Release\Source\paru_assemble.cpp.obj
  FAILED: CMakeFiles/paru.dir/Release/Source/paru_assemble.cpp.obj 

Afaict, the taskloop construct was introduced with OpenMP 4.5. MSVC supports only OpenMP 2.0.

To Reproduce
Try to build ParU with OpenMP using MSVC (see CI) or another toolchain that lacks support for OpenMP 4.5.

Expected behavior
ParU builds without error.

Desktop (please complete the following information):

• OS: Windows
• compiler MSVC 2022

[DrTimothyAldenDavis]

Also tagging @Aznaveh.

The solution for this is to detect if OpenMP is 4.5 or later, and only use it for ParU for that version of OpenMP. If OpenMP is an older version, like OpenMP 2.0 in Microsoft Visual Studio, then ParU needs to be compiled without OpenMP.

So that's a bug in our cmake -- we should be detecting this case in the cmake build system, and not use OpenMP for ParU if the version is too old.

The rest of SuiteSparse is fine with OpenMP 2.0 (CHOLMOD uses it in a few places,and GraphBLAS is fundamentally parallel). I did use tasking in one place in GraphBLAS once, but I was able to change my algorithms to use the more basic OpenMP 2.0 methods.

The parallel algorithms in ParU can't be redesigned, however. They are much more complex than my parallel algorithms in GraphBLAS and the will only work with OpenMP tasking.

[mmuetzel]

Thanks for clarifying. I opened #463 that should fix this.

It looks like building LAGraph with OpenMP also doesn't work.
Error:

  [12/372] Building C object src\CMakeFiles\lagraph.dir\Release\algorithm\LG_CC_FastSV6.c.obj
  FAILED: src/CMakeFiles/lagraph.dir/Release/algorithm/LG_CC_FastSV6.c.obj 
  C:\Miniconda\envs\test\Library\bin\ccache.exe C:\PROGRA~1\MICROS~2\2022\ENTERP~1\VC\Tools\MSVC\1435~1.322\bin\HostX64\x64\cl.exe  /nologo -D_CRT_SECURE_NO_WARNINGS -Dlagraph_EXPORTS -DCMAKE_INTDIR=\"Release\" -ID:\a\SuiteSparse\SuiteSparse\LAGraph\build\src -ID:\a\SuiteSparse\SuiteSparse\LAGraph\src -ID:\a\SuiteSparse\SuiteSparse\LAGraph\include -ID:\a\SuiteSparse\SuiteSparse\LAGraph\src\utility -ID:\a\SuiteSparse\SuiteSparse\LAGraph\test\include -ID:\a\SuiteSparse\SuiteSparse\GraphBLAS\Include -I\usr\local\include -ID:\a\SuiteSparse\SuiteSparse\LAGraph\src\include /DWIN32 /D_WINDOWS /W3 -DLGDIR=D:/a/SuiteSparse/SuiteSparse/LAGraph /MD /O2 /Ob2 /DNDEBUG /MD /O2 /Ob2 /DNDEBUG -std:c11 -openmp /showIncludes /Fosrc\CMakeFiles\lagraph.dir\Release\algorithm\LG_CC_FastSV6.c.obj /Fdsrc\CMakeFiles\lagraph.dir\Release\ /FS -c D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\utility\LG_internal.h(390): warning C4244: '=': conversion from 'double' to 'int64_t', possible loss of data
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c(297): warning C4244: '=': conversion from 'GrB_Index' to 'int', possible loss of data
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c(421): error C3015: initialization in OpenMP 'for' statement has improper form
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c(446): error C3015: initialization in OpenMP 'for' statement has improper form
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c(556): error C3015: initialization in OpenMP 'for' statement has improper form
  D:\a\SuiteSparse\SuiteSparse\LAGraph\src\algorithm\LG_CC_FastSV6.c(611): error C3015: initialization in OpenMP 'for' statement has improper form

Not sure (yet) what that means. "improper form".
A version issue again?

[mmuetzel]

It looks like MSVC requires that source code follows a stricter interpretation of the OpenMP standard.
I opened #467 with changes that allow LAGraph to be compiled with OpenMP from MSVC.

[mmuetzel]

Thanks for merging the PRs.
This seems to be working now.

[DrTimothyAldenDavis]

Thanks for your help!