/openmp:llvm doesn't imply /openmp:experimental

[WRFan]

Issue Description

If I set:

/openmp:experimental

I get:

C3016: 'var' : index variable in OpenMP 'for' statement must have signed integral type

If I set

/openmp:llvm

I get

C7660: "simd": requires "-openmp:experimental" command line option(s)

If I set both flags (like you said above!):

 /openmp:experimental /openmp:llvm

I get:

C4005: "_OPENMP": macro redefinition

The last one is a warning, so at least stuff compiles, but I get tons of warnings.

If I set:

/D _OPENMP /D _OPENMP_LLVM_RUNTIME

the result is not the same as with the real "/openmp:" flags, so this is not a solution. What's "_OPENMP_LLVM_RUNTIME"? Undocumented.

Anything I can set you don't whine about? You first force me to set the _OPENMP flag twice, then warn me about it. We warn you that we forced you! Bah!

Steps to Reproduce

I don't know. Compile something? Like tesseract, for example?

Expected Behavior

What I expect... right. Who designed this template?!

Actual Behavior

above

Analysis

• If you have an idea where the problem might lie

Your brains?

Versions & Configurations

16.9.0+5e4b48a27 16.9.0.16703

14.28.29910 10.0.22000.0

Hope you know what those numbers mean, cause I don't

Windows [Version 10.0.19042.804] x64

[v-zhiyul]

Hi WRFan, for further investigation, could you provide the command line you run when you hit this issue? Or detailed steps to manually reproduce this ourselves? Thanks!

[WRFan]

To reproduce this bug is very easy. Just compile ANY file with BOTH flags set:

/openmp:experimental /openmp:llvm

Let's say you have a C file:

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PSTR szCmdLine, int iCmdShow)
{
}

Now just compile it and you get:

C4005: "_OPENMP": macro redefinition

if both "/openmp" flags are used.

Now maybe I wasn't clear why I want to use both flags during compilation. I mentioned Tesseract. Now take a look at this file:

https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/ccmain/par_control.cpp

#ifdef _OPENMP
    #pragma omp parallel for num_threads(10)
#endif

If you try to compile this file with just "/openmp:experimental" ("/openmp:llvm" NOT set), you get:

C3016: 'var' : index variable in OpenMP 'for' statement must have signed integral type

But these files:

https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/arch/dotproduct.cpp https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/lstm/fullyconnected.cpp https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/lstm/lstm.cpp https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/lstm/parallel.cpp https://raw.githubusercontent.com/tesseract-ocr/tesseract/master/src/lstm/weightmatrix.cpp

specifically require "/openmp:experimental" being set.

#if defined(_OPENMP)
    #pragma omp simd reduction(+ : total)
#endif

So if you just set "/openmp:llvm" (to satisfy the compilation requirement for par_control.cpp - above), "dotproduct.cpp" etc. will throw:

C7660: "simd": requires "-openmp:experimental" command line option(s)

So that's why I set BOTH "/openmp" flags, to avoid the errors on all files. But since the compiler (c1xx.dll) sets the "_OPENMP" macro for each "/openmp" directive passed to it, this results in a macro re-definition I mentioned above:

C4005: "_OPENMP": macro redefinition

I could of course disable the warning altogether:

/wd4005

but that would suppress ALL macro redefinition warnings, not just this particular one, and that won't do.

Now you might ask me why I don't set the compiler flags individually for each source file. Sure, I could do that:

Using cmake:

set(BuildTargets
    src/dotproduct.cpp
    src/fullyconnected.cpp
    src/lstm.cpp
    src/parallel.cpp
    src/weightmatrix.cpp
)

foreach(obj ${BuildTargets})
    set_source_files_properties(${obj} PROPERTIES COMPILE_FLAGS "/openmp:experimental")
endforeach()

set(BuildTargets
    src/legacy/par_control.cpp
    src/tesseractmain.cpp
)

foreach(obj ${BuildTargets})
    set_source_files_properties(${obj} PROPERTIES COMPILE_FLAGS "/openmp:llvm")
endforeach()

And then build it:

set CompilerFlags=/D _WINDOWS /D _WIN32_WINNT=0x0a00 /D WINVER=0x0a00 /D NDEBUG /D _CRT_SECURE_NO_WARNINGS /D _AFXDLL /W3 /utf-8 /GR- /EHsc /nologo /FC /Oi /Ot /Ob3 /GF /Gy /favor:INTEL64 /GL /cgthreads8 /GS- /wd4101 /we4013 /I\"%programfiles%/visual studio/windows kits/10/include/10.0.22000.0/km\" /MP /arch:AVX /arch:AVX2 /D __MMX__ /D __SSE__ /D __SSE2__ /D __SSE3__ /D __SSSE3__ /D __SSE4_1__ /D __SSE4_2__ /D __FMA__ /D __BMI2__ /std:c17 /std:c++latest /Zc:__cplusplus

set LinkerFlags=/INCREMENTAL:NO /OPT:REF,ICF /LTCG /MANIFESTUAC:NO /DYNAMICBASE:NO /LIBPATH:\"%programfiles%/visual studio/windows kits/10/lib/10.0.22000.0/km/x64\" /NODEFAULTLIB:libcmt

cmake .. -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_CONFIGURATION_TYPES:STRING=Release -DCMAKE_C_FLAGS:STRING="%CompilerFlags%" -DCMAKE_CXX_FLAGS:STRING="%CompilerFlags%" -DCMAKE_C_FLAGS_RELEASE:STRING="" -DCMAKE_CXX_FLAGS_RELEASE:STRING="" -DCMAKE_EXE_LINKER_FLAGS_RELEASE:STRING="%LinkerFlags%" -DCMAKE_SHARED_LINKER_FLAGS_RELEASE:STRING="%LinkerFlags%" -DCMAKE_BUILD_TYPE=Release -DCMAKE_CONFIGURATION_TYPES=Release

cmake --build . --config Release

This results in these msbuild directives (as created by cmake):

<AdditionalOptions>
    %(AdditionalOptions) /utf-8 /Ob3 /favor:INTEL64 /cgthreads8 /Zc:__cplusplus
</AdditionalOptions>

<ClCompile Include="C:\Copy\Packs\tesseract\src\legacy\par_control.cpp">
    <AdditionalOptions Condition="'$(Configuration)|$(Platform)'=='Release|x64'">
        %(AdditionalOptions) /openmp:llvm
    </AdditionalOptions>
</ClCompile>

...
etc.

But then I would have to adjust the compiler flags for EACH single source file, and that for EACH single project I build, meaning I would have more work to do so YOU have less. I think we do it the other way around - you just fix the bug, then YOU do the work so we can simply set both "/openmp" flags without being nagged by those "macro redefinition" warnings.

Why are there two "/openmp" flags anyway? Why is the "/openmp:experimental" not sufficient to compile ANY source file containing "#pragma omp"? If somebody wants to use openmp, then they obviously want to use it on all files in a project, i.e., a single flag would suffice to enable/disable openmp? If both flags should still be necessary for some reason, the compiler should set the "_OPENMP" macro only once, even if both flags are there. I can't find out where this macro is being set, otherwise I'd fix it myself. It's not set by SDK header files as far as I can see, it must be compiled into c1xx.dll.

I don't know if this is the right repository to post about this issue, since technically, this is a compiler issue, not an msbuild one, but I don't work for MS, I don't know your internal group structure. If your group is responsible for msbuild only, then notify the people responsible for the MSVC compiler.

[v-zhiyul]

@WRFan Thanks for your reply! According to your description, it looks like this isn't an MSBuild issue. You can open a feedback for compiler(via your VS -> clicking "Help" -> "Send Feedback" -> "Report a Problem.." or "https://developercommunity.visualstudio.com/spaces/8/index.html").

[benvillalobos]

Why is the "/openmp:experimental" not sufficient to compile ANY source file containing "#pragma omp"?

@yuehuang010 can you help with routing this, or is a feedback ticket the way to go here?

[benvillalobos]

Created a feedback ticket for this: https://developercommunity.visualstudio.com/t/openmp:llvm-doesnt-imply-openmp:exper/1520948.

[benvillalobos]

@WRFan there's activity on the feedback ticket, if you reply there we can help solve your issue. Thanks!

https://developercommunity.visualstudio.com/t/openmpllvm-doesnt-imply-openmpexperimental/1520948?from=email&viewtype=all#T-ND1522460

[peekxc]

This does not appear to be fixed.

[rainersigwald]

@peekxc but it's still not an MSBuild bug. The right place to track is still on the Developer Community site.