Enable -Werror for MIOpen by replacing -Weverything with -Wall -Wextra#7257
Draft
BradPepersAMD wants to merge 12 commits intodevelopfrom
Draft
Enable -Werror for MIOpen by replacing -Weverything with -Wall -Wextra#7257BradPepersAMD wants to merge 12 commits intodevelopfrom
BradPepersAMD wants to merge 12 commits intodevelopfrom
Conversation
…IOpen The MIOpen build with -Weverything produces ~31K warnings, almost all from external HIP runtime and rocRAND headers. Add targeted -Wno-* suppressions to EnableCompilerWarnings.cmake for warning categories that only fire on external headers, eliminating ~90% of the noise. Also fix a macro redefinition warning where hip_float8.hpp defaulted MIOPEN_FP8_IEEE_EXPONENT_BIAS to 1 before config.h could set it to the CMake-configured value (0). Use __has_include to pull config.h when available (host compilation), while preserving the fallback for kernel compilation where config.h is not in the include path. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
…ludes as SYSTEM Switch MIOpen from clang's -Weverything (which enables every possible warning and required 30+ suppressions) to the standard -Wall -Wextra plus three explicitly-enabled warnings not covered by those flags: -Wundef, -Wunreachable-code, and -Wmissing-noreturn. This eliminates all the -Wno-* suppressions that were only needed to counteract -Weverything, reducing the warning configuration from ~70 lines to ~30 lines. The remaining suppressions (-Wno-ignored-qualifiers, -Wno-sign-compare, -Wno-deprecated-declarations, -Wno-deprecated, -Wno-unused-command-line-argument) target specific -Wall/-Wextra warnings. Also mark hipBLAS-common and rocRAND include directories as SYSTEM in src/CMakeLists.txt so that warnings from external headers are suppressed by the compiler rather than requiring per-warning-type suppressions. With -Werror now enabled, any new warning in MIOpen's own code will cause a build failure, preventing warning regressions. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
MIOpen has several int-to-size_t and double-to-float narrowing conversions in batchnorm solvers, CK utilities, and generated kernel data files. Suppress these with -Wno-c++11-narrowing to allow -Werror to be enabled without breaking the build. The narrowing issues can be addressed individually in follow-up work. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Guard WIN32_LEAN_AND_MEAN defines with #ifndef in env.cpp and file_lock.hpp to prevent redefinition errors, since the macro is already defined via CMake compile flags (-DWIN32_LEAN_AND_MEAN). Disable -Wundef on Windows builds because external headers (rocRAND, HIP) are not consistently included as system headers on Windows, causing -Wundef errors from undefined macros like __HIP_DEVICE_COMPILE__ in rocrand_common.h. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Add -Wshadow (was active and clean under old -Weverything config). Remove -Wno-ignored-qualifiers and -Wno-deprecated (zero instances). Document in EnableCompilerWarnings.cmake: - Warnings we aspire to enable: -Wconversion, -Wold-style-cast, -Wsuggest-override, -Wdouble-promotion (with effort estimates) - Why each remaining suppression exists and how many instances need fixing to remove it Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Enable -Wshadow (was active under old -Weverything, MIOpen is clean). Enable -Wsuggest-override and add missing override specifiers to SolverDbId, GetDefaultPerformanceConfig, IsValidPerformanceConfig, and Search methods in transposed conv/pooling solver wrappers. Remove -Wno-ignored-qualifiers and -Wno-deprecated (zero instances in codebase, dead weight). Document aspirational warnings (-Wconversion, -Wold-style-cast, -Wdouble-promotion) and existing suppressions with instance counts to guide future cleanup. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Replace 7 C-style casts with static_cast in batchnorm, implicit gemm, and kthvalue driver code. Enable -Wold-style-cast to prevent new C-style casts from being introduced. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Fix old-style casts and deprecated C++20 lambda captures found when building test targets (previously EXCLUDE_FROM_ALL, now built via 'ninja tests'): - test/gtest/kthvalue.hpp: (size_t*) -> static_cast<size_t*> - test/gtest/na_inference_find2.cpp: (void*) -> static_cast<void*> - test/gtest/rnn_seq_api.hpp: [=] -> [=, this] in member function lambda - test/driver.hpp: [=] -> [=, this] for member function lambdas - test/conv_common.hpp: [=] -> [=, this] for member function lambdas Fix CI -Wundef from rocrand headers: in TheRock builds, rocrand exposes both stage/include (non-system) and dist/include via its imported cmake target. Add a foreach loop in src/CMakeLists.txt to detect the rocrand imported target and mark its INTERFACE_INCLUDE_DIRECTORIES as SYSTEM, so rocrand_common.h's bare #if __HIP_DEVICE_COMPILE__ doesn't trigger -Werror,-Wundef during host compilation. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
rocrand_wrapper.cpp includes <rocrand/rocrand.hpp> directly, so MIOpenDriver is exposed to the same rocrand stage/include -Wundef issue as the MIOpen library target. Apply the same imported-target SYSTEM include workaround that was added to src/CMakeLists.txt in the previous commit. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
rocrand_common.h uses bare #if __HIP_DEVICE_COMPILE__ (not #ifdef), which triggers -Wundef when included via a non-system path. In the TheRock build system, rocRAND/stage/include is injected as a global -I flag at the build-system level — not via rocrand's CMake imported target — so it cannot be overridden to -isystem from within MIOpen's CMakeLists. Remove -Wundef from the warning set and document the root cause. Also remove the now-unnecessary roc::rocrand INTERFACE_INCLUDE_DIRECTORIES SYSTEM workarounds from src/ and driver/ CMakeLists, and remove the WIN32-only -Wno-undef suppression (no longer needed since -Wundef is gone). Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…ites TheRock injects rocRAND/stage/include globally as -I (not -isystem), causing rocrand's own headers to trigger -Wold-style-cast and -Wundef under our strict warning flags. Rather than disabling these warnings globally, suppress them only at the two points where rocrand headers are included: - src/kernels/miopen_rocrand.hpp: wraps <rocrand/rocrand_xorwow.h> - driver/rocrand_wrapper.cpp: wraps <rocrand/rocrand.hpp> Both use #pragma clang diagnostic push/pop so the suppression is scoped tightly to the rocrand include and all MIOpen code continues to be checked. Re-add -Wundef and -Wold-style-cast to EnableCompilerWarnings.cmake. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-Weverything(which required 30+-Wno-*suppressions) with-Wall -Wextra -Werrorplus the following explicitly-enabled warnings (-Wundef,-Wunreachable-code,-Wmissing-noreturn,-Wshadow,-Wsuggest-override,-Wold-style-cast)SYSTEMinsrc/CMakeLists.txtto suppress warnings from external headers at the compiler level#pragma clang diagnostic push/poparound rocRAND includes insrc/kernels/miopen_rocrand.hppanddriver/rocrand_wrapper.cppto handle TheRock builds where rocRAND headers are injected as plain-I(not-isystem) and trigger-Wold-style-castand-WundefMIOPEN_FP8_IEEE_EXPONENT_BIASmacro redefinition warning by using__has_includeinhip_float8.hpptest/gtest/-Wno-c++11-narrowing,-Wno-sign-compare,-Wno-deprecated-declarationsThe warning configuration goes from ~70 lines to ~15 lines, and any new warning in MIOpen's own code will now break the build.
Risk and mitigation
This PR is high risk. Enabling
-Werrormeans any compiler warning — including warnings from transitive includes or platform-specific code paths — will break the build entirely. It is not feasible to test every combination of build flags, GPU targets, and platforms before merging.Mitigation strategy:
rocm-librarieswith labels to include all GPU targets, covering the broadest possible build matrix before mergeTest plan
-Werrorproduces zero warnings and zero errorsSYSTEMfrom rocRAND include dirs) produces zero warnings🤖 Generated with Claude Code