Using Werror-all for Intel

[philbucher]

Description

For turning compiler warnings into errors with Intel I think it is necessary to use Werror-all instead of Werror
Source

EDIT Werror exists too, but Werror-all is more strict

I think there will be some minor things to be fixed, at least I had to do this in my project.
Lets see if the CI picks it up :)

Suggested changelog entry:

Changed ``Werror`` to stricter ``Werror-all`` for Intel compiler and fixed minor issues

[philbucher]

ok the CI found the same issues I had in my code:

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_cross_module_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(147): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic push
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(147): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic push
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_cross_module_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(148): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic ignored "-Wplacement-new"
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_cross_module_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(152): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic pop
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(148): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic ignored "-Wplacement-new"
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(152): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic pop
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_cross_module_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(2287): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic pop
                           ^

In file included from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.h(2),
                 from /home/runner/work/pybind11/pybind11/tests/pybind11_tests.cpp(10):
/home/runner/work/pybind11/pybind11/include/pybind11/pybind11.h(2287): error #2282: unrecognized GCC pragma
  #  pragma GCC diagnostic pop
                           ^

my second commit should fix them

[henryiii]

Is this true for both Intel compilers? (IntelLLVM and classic Intel; both will match here)

[philbucher]

thx for the fast reply

No clue tbh. This is the first time I learned about IntelLLVM. I took a brief look at their docs but couldn't find it

Are you sure it would match both? According to CMake the CMAKE_CXX_COMPILER_ID for IntelLLVM is IntelLLVM

also checking the docs more in detail I noticed that Werror does exist, but it is not as strict as Werror-all. Maybe Werror-all is too strict? What do you think

[henryiii]

MATCH will find the word anywhere; Clang and AppleClang MATCH. So IntelLLVM will MATCH Intel. (I guess they chose LLVM to avoid MATCHing with Clang, as IntelClang would have matched, and due to the AppleClang compiler, that's a very common expression in CMakeLists.

The Clang-based Intel compiler is the "new" compiler, and the one they are supporting going forward. The old compiler is now renamed "Classic Intel" (as you can see in the docs), and is being killed off.

[henryiii]

The new compiler is called "Intel® oneAPI DPC++/C++ Compiler", the old one "Intel® C++ Compiler Classic", as you can see here: https://software.intel.com/content/www/us/en/develop/tools/oneapi/all-toolkits.html

[philbucher]

thx for the explanation.
I used STREQUAL now, please let me know if it is ok

[henryiii]

Ignore the unrelated failure for Python 3.10, that's due to pytest-dev/pytest#8539

[philbucher]

so now the ward issues are fixed, but there are two more warnings:

error #11074: Inlining inhibited by limit max-size 
error #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo

I am not sure what to do about those. Maybe there are some limits or inlining specified? => see here

[henryiii]

I think we set something similar for the NVHPC (PGI) compiler, which I think shares some history with Intel (IIRC). -no-inline-max-size might force it? Though from what I can tell, it uses default heuristics, and that probably causes a few requested inlines to not happen (and hence, the warning becoming an error above). Maybe we can find the size with -qopt-report=4 -qopt-report-phase ipo and then set it a little larger than that with -max-size?

[rwgk]

EDIT 4/14: I was wrong before (below), I overlooked that this PR does not affect the pragma block.

Note to myself: the smart_holder branch will need to be updated accordingly.

For the smart_holder branch I had to move the pragma block in pybind11.h (#2873), which wasn't supported by anyone unfortunately.

[philbucher]

I think we set something similar for the NVHPC (PGI) compiler, which I think shares some history with Intel (IIRC). -no-inline-max-size might force it? Though from what I can tell, it uses default heuristics, and that probably causes a few requested inlines to not happen (and hence, the warning becoming an error above). Maybe we can find the size with -qopt-report=4 -qopt-report-phase ipo and then set it a little larger than that with -max-size?

this is what I get when enabling the report:

INLINING OPTION VALUES:
  -inline-factor: 100
  -inline-min-size: 30
  -inline-max-size: 230
  -inline-max-total-size: 2000
  -inline-max-per-routine: disabled
  -inline-max-per-compile: disabled

I then thought it would probably be better to disable the warnings for the following reasons:

• The log is very large (~500K lines). Parsing it to find the max numbers is possible but I think it is dangerous if in the future the size increases further then suddenly the intel build will fail for a seemingly unrelated change
• I also thought of removing the limits completely but this is
  • a behavior change
  • can lead to large memory consumption (which I already fought with quite a lot with GitHub Actions)
• the limits for inlining are already used, but they werent noticed as the compiler didn't issue the warning

Bottom line now I disabled them, which I think is the best solution

Thx for your help, please let me know what you think

[philbucher]

cool now the CI passes (except the build you said doesn't work)

[rwgk]

Looks good to me.

[philbucher]

thanks for the fast responses and help!

[henryiii]

Thanks!