No rtti headers - #477
Conversation
Graph::register_oss_engine_() and Graph::register_oss_rms_norm_silu_engine_()
use dynamic_cast to locate nodes in sub_nodes. Both are inline members of
Graph, so every translation unit that includes cudnn_frontend.h compiles
them, and GCC/Clang reject the header outright when RTTI is disabled:
graph_interface.h:425:35: error: 'dynamic_cast' not permitted with '-fno-rtti'
This makes the headers unusable for any consumer building with -fno-rtti or
/GR-, a common configuration for libraries that ship binaries. It has been
the case since these engines were introduced in v1.19.0.
MSVC does not error, so the problem is invisible on Windows: it emits C4541
("unpredictable behavior may result") and compiles. RTTI-disabled Windows
builds therefore reach these casts with no guarantee they behave correctly.
Replace both cast sites with RTTI-free equivalents:
- SDPA lookup: add a virtual INode::get_sdpa_attributes() returning nullptr
by default, overridden once in SDPANodeBase. CompositeSDPANode and
UnifiedSDPANode both inherit `attributes` from that base, so a single
override covers both and the two cast branches collapse into one.
- RMSNorm+SiLU pattern match: gate on getType() and static_cast. RMSNORM and
POINTWISE are distinct Type values, so this is an exact substitute for the
check the dynamic_casts performed.
Both replacements are cheaper than the casts they replace: a virtual
dispatch and an enum comparison rather than an RTTI walk.
The static_cast downcasts are sound. NodeCRTP derives from INode via public
non-virtual single inheritance, and NodeCRTP already relies on the same
property internally via static_cast<DerivedT*>(this).
No functional change for RTTI-enabled builds.
Adds CUDNN_FRONTEND_ENABLE_RTTI (default OFF), which passes -fno-rtti (GCC/Clang) or /GR- (MSVC) to samples and tests, so a dynamic_cast added to the headers fails the build instead of only breaking downstream consumers that disable RTTI. The python bindings opt back in: pybind11's type registry is typeid-based and requires RTTI.
📝 WalkthroughWalkthroughThe build adds configurable RTTI support. General builds can disable RTTI, while the Python module enables it for pybind11. Graph inspection uses virtual SDPA attributes and explicit node type checks instead of RTTI casts. ChangesRTTI graph support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CMakeLists.txt`:
- Around line 13-15: Remove the inherited /GR- from the Python _compiled_module
configuration: update CMakeLists.txt lines 13-15 and python/CMakeLists.txt lines
75-81 so /GR- is scoped to non-Python targets or removed before _compiled_module
is created, while preserving RTTI-disabled behavior for other MSVC targets.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cf66e3ae-e879-413f-89b0-49386e01b2cf
📒 Files selected for processing (5)
CMakeLists.txtinclude/cudnn_frontend/graph_interface.hinclude/cudnn_frontend/node/scaled_dot_product_flash_attention.hinclude/cudnn_frontend/node_interface.hpython/CMakeLists.txt
| if(NOT CUDNN_FRONTEND_ENABLE_RTTI) | ||
| add_compile_options(/GR-) | ||
| endif() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CMakeLists.txt ---'
sed -n '1,90p' CMakeLists.txt
printf '%s\n' '--- python/CMakeLists.txt ---'
sed -n '1,125p' python/CMakeLists.txt
printf '%s\n' '--- RTTI and warning-as-error settings ---'
rg -n --glob 'CMakeLists.txt' --glob '*.cmake' --glob '*.cmake.in' \
'CUDNN_FRONTEND_ENABLE_RTTI|/GR-|/GR\b|-fno-rtti|-frtti|/WX|COMPILE_WARNING_AS_ERROR|target_compile_options|add_compile_options' .
printf '%s\n' '--- relevant targets and directory structure ---'
rg -n --glob 'CMakeLists.txt' 'add_(executable|library)|pybind|_compiled_module|add_subdirectory\(python|enable_testing|CUDNN_FRONTEND_BUILD' .Repository: NVIDIA/cudnn-frontend
Length of output: 11437
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- CMakeLists.txt ---'
sed -n '1,90p' CMakeLists.txt
printf '%s\n' '--- python/CMakeLists.txt ---'
sed -n '1,125p' python/CMakeLists.txt
printf '%s\n' '--- RTTI and warning-as-error settings ---'
rg -n --glob 'CMakeLists.txt' --glob '*.cmake' --glob '*.cmake.in' \
'CUDNN_FRONTEND_ENABLE_RTTI|/GR-|(\/GR)([^-]|$)|-fno-rtti|-frtti|/WX|COMPILE_WARNING_AS_ERROR|target_compile_options|add_compile_options' .
printf '%s\n' '--- relevant targets and directory structure ---'
rg -n --glob 'CMakeLists.txt' \
'add_(executable|library)|pybind|_compiled_module|add_subdirectory\(python|enable_testing|CUDNN_FRONTEND_BUILD' .Repository: NVIDIA/cudnn-frontend
Length of output: 11437
🌐 Web query:
site:cmake.org/cmake/help/latest add_compile_options target_compile_options directory COMPILE_OPTIONS initialized target
💡 Result:
In CMake, compile options can be managed at different scopes, ranging from directory-wide to target-specific, using the following mechanisms: add_compile_options The add_compile_options command operates at the directory scope [1][2]. It adds options to the COMPILE_OPTIONS directory property [1]. These options are automatically applied to all targets created within that directory and any subdirectories processed afterward [1]. target_compile_options The target_compile_options command operates at the target scope [3][2]. It adds options to the COMPILE_OPTIONS and/or INTERFACE_COMPILE_OPTIONS target properties of a specific target [3][4]. This command requires a scope keyword (PRIVATE, PUBLIC, or INTERFACE) to determine whether the options should only apply to the target itself (PRIVATE), be propagated to consumers (INTERFACE), or both (PUBLIC) [3][4]. Initialization and Property Relationships The COMPILE_OPTIONS target property is initialized using the current value of the COMPILE_OPTIONS directory property at the time the target is created [5][6]. Once a target is created, the directory-level options and the target-level options are distinct; subsequent changes to the directory property will not affect existing targets [5]. When building a target, the final set of compile options consists of the target's own COMPILE_OPTIONS property (which includes its initial directory-based options plus any added via target_compile_options(PRIVATE/PUBLIC)) and the INTERFACE_COMPILE_OPTIONS properties propagated from its dependencies [5][7][4].
Citations:
- 1: https://cmake.org/cmake/help/latest/command/add_compile_options.html
- 2: https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html
- 3: https://cmake.org/cmake/help/latest/command/target_compile_options.html
- 4: https://cmake.org/cmake/help/latest/command/target%5Fcompile%5Foptions.html
- 5: https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_OPTIONS.html
- 6: https://cmake.org/cmake/help/latest/prop_dir/COMPILE_OPTIONS.html
- 7: https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html?highlight=imported%20target
🌐 Web query:
site:learn.microsoft.com MSVC D9025 overriding /GR- /GR /WX warning
💡 Result:
The MSVC compiler warning D9025 (overriding 'option1' with 'option2') occurs when conflicting command-line options are provided [1][2]. In the case of /GR and /GR-, these options are mutually exclusive: /GR enables Run-Time Type Information (RTTI), while /GR- disables it [3][4]. The compiler resolves this conflict by using the option that appears farthest to the right on the command line [1]. If both are specified, the one encountered last overrides the previous one [1]. Common causes for this warning include: 1. Conflicting Project Settings: The options may be defined in different places, such as the project's global C/C++ property pages and a specific source file's individual properties [1]. 2. Command Line Arguments: Build systems or manual command lines might inadvertently include both flags [1]. 3. Preprocessor or Build Tools: Generated build files (e.g., from CMake) may include conflicting configurations [5]. To resolve this: - Examine the compiler's Command Line property pages to see the full list of options applied [1]. - Check for redundant or contradictory settings in the C/C++ Language property page of your project [1][3]. - If you are using a build system like CMake, verify the configuration to ensure the desired RTTI setting is applied consistently across the project [5]. - If you need to override a project-wide setting, check the Additional Options field in the Command Line property page; adding the desired flag there will place it later on the command line, effectively overriding previous settings [1][6]. Note that /WX (Treat Warnings as Errors) is a separate flag that causes the compiler to treat all warnings as errors, which may cause your build to fail if a D9025 warning is generated [2]. You must resolve the underlying option conflict to prevent the D9025 warning, which in turn will resolve the /WX failure [1][2].
Citations:
- 1: https://learn.microsoft.com/en-us/cpp/error-messages/tool-errors/command-line-warning-d9025?view=msvc-170
- 2: https://learn.microsoft.com/en-us/cpp/error-messages/tool-errors/command-line-errors-d8000-through-d9999?view=msvc-170
- 3: https://learn.microsoft.com/en-us/cpp/build/reference/gr-enable-run-time-type-information?view=msvc-170
- 4: https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2008/we6hfdy0(v=vs.90)
- 5: https://learn.microsoft.com/en-us/answers/questions/2109389/cmake-build-cef-with-vs2022
- 6: https://learn.microsoft.com/en-us/cpp/build/reference/fa-fa-listing-file?view=msvc-170
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
root = Path(".")
top = (root / "CMakeLists.txt").read_text()
py = (root / "python" / "CMakeLists.txt").read_text()
checks = {
"MSVC global no-RTTI option": bool(re.search(
r'if\(NOT CUDNN_FRONTEND_ENABLE_RTTI\).*?add_compile_options\(/GR-\)',
top, re.S)),
"Python target is created in the Python subdirectory": "python_add_library(" in py
and "_compiled_module" in py,
"Python MSVC RTTI option": bool(re.search(
r'if\(MSVC\).*?target_compile_options\(_compiled_module PRIVATE /GR\)',
py, re.S)),
"global options are applied before Python subdirectory": top.index(
"add_compile_options(/GR-)") < top.index("add_subdirectory(python)"),
}
for name, value in checks.items():
print(f"{name}: {'yes' if value else 'no'}")
assert all(checks.values())
# CMake's directory-property initialization means the target receives /GR-
# before target_compile_options(PRIVATE /GR) adds /GR.
expected_msvc_options = ["/GR-", "/GR"]
print("Expected MSVC RTTI option sequence for _compiled_module: "
+ " ".join(expected_msvc_options))
print("D9025 condition: yes (mutually exclusive RTTI options)")
print("CUDNN_FRONTEND_ENABLE_RTTI=ON conflict: no (/GR- is not added)")
PYRepository: NVIDIA/cudnn-frontend
Length of output: 520
Remove the inherited /GR- from _compiled_module.
When CUDNN_FRONTEND_ENABLE_RTTI=OFF on MSVC, the target receives both /GR- and /GR. MSVC emits D9025, and /WX can fail the build. Scope /GR- to non-Python targets or remove it from the Python directory before creating _compiled_module.
📍 Affects 2 files
CMakeLists.txt#L13-L15(this comment)python/CMakeLists.txt#L75-L81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CMakeLists.txt` around lines 13 - 15, Remove the inherited /GR- from the
Python _compiled_module configuration: update CMakeLists.txt lines 13-15 and
python/CMakeLists.txt lines 75-81 so /GR- is scoped to non-Python targets or
removed before _compiled_module is created, while preserving RTTI-disabled
behavior for other MSVC targets.
Source: MCP tools
|
@cudnn-ci-bot run |
|
🚀 Running mirror pipeline Branch: cudnn-gh/pr-477-597a3d8 |
Before submitting
pre-commit runand committed any formatting changes.Affected area
Summary
Replace dynamic_cast usage in the public headers.
Why
Support for consuming applications that use no-rtti.
Related issues
API and compatibility impact
Testing
Summary by CodeRabbit
New Features
Compatibility