Enable BinSkim-compliant compile flags#2335
Conversation
apwojcik
force-pushed
the
enable_binskim_compliant_flags
branch
2 times, most recently
from
78144b1 to
30b7d08
Compare
apwojcik
force-pushed
the
enable_binskim_compliant_flags
branch
from
30b7d08 to
a251b4d
Compare
| add_compile_definitions(_SILENCE_NONFLOATING_COMPLEX_DEPRECATION_WARNING) | ||
| endif() | ||
| endif() | ||
| set(LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "" FORCE) |
There was a problem hiding this comment.
Q: Why is it forced to be OFF for all builds ?
There was a problem hiding this comment.
On Windows, to enable assertions, LLVM forces _DEBUG even in Release builds (assert support macros and functions are not available in other build types than Debug). On the other hand, keeping only for Debug builds requires a lot of work. For the multi-config generators (such as MSVC), you cannot just use CMAKE_BUILD_TYPE because it is not used for them. One needs to use generator expressions, but the set() command does not evaluate the expression. It only stores a text as is in a variable.
The easiest way is to turn them off for all build types. BTW, by default, the Clang build scripts are setting assertions off, too.
| add_compile_options( | ||
| -Wno-sign-compare | ||
| -Wno-covered-switch-default | ||
| -Wno-unused-result | ||
| -Wno-switch | ||
| -Wno-language-extension-token | ||
| -Wno-microsoft-enum-value | ||
| -Wno-undef | ||
| -Wno-deprecated-declarations | ||
| -Wno-cast-function-type-mismatch | ||
| -Wno-overlength-strings) |
There was a problem hiding this comment.
Why all these warnings are being silenced ? It can suppress some useful debugging warnings/info
There was a problem hiding this comment.
Those are required only when compiled with Clang on Windows. With the Clang GNU command-line interface, specifically. We recently switched to Clang-Cl, and those may not be required. I will double-check and submit a PR to remove all that is not required from this list.
If we do not switch those warnings off, the screen is flooded with warning messages. It is very hard to sort out a real problem between them. Most of the warnings are due to GNU Clang using MSVC standard libraries, which are not 100% compatible with GNU Clang.
When compiled with MSVC (the default on Windows), those warnings are not presented.
| cmake_policy(SET CMP0057 NEW) | ||
|
|
||
| if(CMAKE_SYSTEM_NAME STREQUAL "Windows") | ||
| option(ROCMLIR_SILENCE_WARNINGS "Ignore certain warnings on Windows" ON) |
There was a problem hiding this comment.
Silencing warnings by default doesn't seem like a good idea. Why is it done this way ?
There was a problem hiding this comment.
We've done it because otherwise we'd be flooded with warnings when compiling with GNU Clang on Windows. I'll try with clang-cl (the Clang with MSVC interface) and let you know if we still need it.
2 participants
PR from the series that is migrating changes from uai-develop to the develop branch.
The PR adds CMake options to enable compilation of rocMLIR on Windows with Spectre mitigated libraries and to add GuardControl to the resulting binaries required for the BinSkim analysis.
Also, it fixes a bug that prevents the rocMLIR compilation on Windows in the Release mode.