Move PropagateCastOps metadata to compile time#29648
Open
GopalakrishnanN wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves PropagateCastOps operator argument metadata from runtime-initialized hash maps to a single compile-time (constexpr) descriptor table, enforcing metadata consistency via compile-time validation and adding targeted tests to prevent policy drift.
Changes:
- Replaced runtime
InlinedHashMapmetadata tables inPropagateCastOpswith aconstexprkRelevantOpArgstable and aFindRelevantOpArgslookup helper. - Added
consteval/static_assertvalidation of metadata invariants (sorted unique op names, valid non-empty index sets, no duplicate indices). - Added a new unit test that performs an exhaustive compile-time truth-table check plus a small runtime lookup test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| onnxruntime/core/optimizer/propagate_cast_ops.cc | Switches relevance checks to use the new compile-time metadata lookup. |
| onnxruntime/core/optimizer/propagate_cast_ops_metadata.h | Introduces the compile-time metadata table, validation, and lookup helper for relevant input/output indices. |
| onnxruntime/test/optimizer/propagate_cast_ops_metadata_test.cc | Adds compile-time and runtime tests to lock in the expected metadata behavior. |
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
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.
Description
InlinedHashMaptables used byPropagateCastOpswith oneconstexproperator argument descriptor table.constevalchecks.Motivation and Context
The relevant input/output policy is fixed program metadata, but it was represented by two separately initialized runtime hash maps. Consolidating it removes duplicate operator keys, makes policy drift a compile-time failure, and avoids runtime hash-table initialization and allocation.
Unknown operators still use the existing fallback where every input/output is relevant.
Measurement
Windows RelWithDebInfo, MSVC 19.51, touched-file rebuild of
propagate_cast_ops.cc:propagate_cast_ops.objonnxruntime_optimizer.libThe single baseline rebuild was 10.83 s; post-change samples were 13.45 s, 12.37 s, and 12.06 s. Build timing is noisy and this PR does not claim a compile-time improvement.
Testing
cmake --build build/Windows/RelWithDebInfo --config RelWithDebInfo --target onnxruntime_test_all --parallel 8onnxruntime_test_all.exe --gtest_filter=PropagateCastOpsMetadataTest.*:GraphTransformationTests.PropagateCastOpsTests*from the test output directory: 2 tests from 2 suites passed.lintrunner -aon all three changed files: no lint issues.