Skip to content

Enhance symbolic onnx parsing via sym_eval - #5148

Open
shivadbhavsar wants to merge 33 commits into
developfrom
core_symbolic_evaluation
Open

Enhance symbolic onnx parsing via sym_eval#5148
shivadbhavsar wants to merge 33 commits into
developfrom
core_symbolic_evaluation

Conversation

@shivadbhavsar

Copy link
Copy Markdown
Contributor

No description provided.

CharlieL7 and others added 15 commits August 4, 2026 18:32
Bring the PR #5112-based parser prerequisite branch onto the current development baseline before applying its remaining changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the merged resolver regression compatible with the dynamic-slice interface introduced by PR #5112.

Co-authored-by: Cursor <cursoragent@cursor.com>
Track exact integral shape values through parser operations so dynamic consumers retain symbolic output relationships without changing runtime dataflow.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the symbolic-value change focused by restoring existing resolver diagnostics and simplifying the signed Gather size declaration.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread src/include/migraphx/op/add.hpp Outdated
Comment thread src/include/migraphx/symbolic_tensor_value.hpp Outdated
Comment thread src/include/migraphx/op/add.hpp Outdated
Comment thread src/instruction.cpp Outdated
Comment thread src/instruction.cpp Outdated
Comment thread src/instruction.cpp Outdated
Comment thread src/instruction.cpp Outdated
Comment thread src/onnx/include/migraphx/onnx/op_parser.hpp Outdated
Comment thread src/sym.cpp Outdated
@shivadbhavsar
shivadbhavsar requested a review from pfultz2 August 21, 2026 01:03
@shivadbhavsar
shivadbhavsar marked this pull request as ready for review August 21, 2026 01:03
@shivadbhavsar
shivadbhavsar requested review from a team and causten as code owners August 21, 2026 01:03
Copilot AI lite review requested due to automatic review settings August 21, 2026 01:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends MIGraphX’s symbolic shape machinery by introducing symbolic tensor evaluation (sym_eval / symbolic_compute) and using it in ONNX parsing and attribute normalization so shape-dependent subgraphs can preserve symbolic output shapes instead of falling back to range-dynamic shapes.

Changes:

  • Add sym_argument + operation::symbolic_compute() and instruction::sym_eval() to enable symbolic evaluation of int64/bool “metadata tensors” (e.g., shapes, indices).
  • Enhance ONNX parsers (e.g., Slice/Reshape/Expand/Range/Trilu/Where/Shape) to leverage sym_eval for improved symbolic output-shape propagation.
  • Add the new dyn_slice operator and symbolic attribute normalization, with broad test coverage across ref/gpu/onnx.

Reviewed changes

Copilot reviewed 68 out of 71 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/include/operation.hpp Add sym_argument include + type-erased symbolic_compute plumbing in generator template.
test/sym.cpp Add rewrite + interval/fixed-value/provable-equality tests for symbolic reasoning improvements.
test/sym_test.cpp Extend sym serialization tests (bare numbers → literals) and reformat a few cases.
test/sym_eval_test.cpp New unit tests for instruction::sym_eval() and symbolic compute fallback behavior.
test/ref/slice.cpp Add symbolic slice coverage and update dynamic slicing test to use dyn_slice.
test/ref/dynamic_range.cpp Add symbolic-output coverage for dynamic_range.
test/ref/dyn_slice.cpp New reference backend tests for dyn_slice semantics and symbolic shapes.
test/onnx/verify/trilu_dynamic_test.cpp New ONNX verify tests for symbolic Trilu output shapes.
test/onnx/verify/symbolic_shape_values_test.cpp New ONNX verify test ensuring symbolic shape metadata is preserved and evaluated at runtime.
test/onnx/verify/symbolic_shape_arithmetic_test.cpp New ONNX verify test for symbolic arithmetic propagation through shape subgraphs.
test/onnx/parse/symbolic_reshape_markers_test.cpp New parse test to assert specific marker pattern for symbolic reshape handling.
test/onnx/gen_onnx.py Add ONNX model generators for new symbolic shape tests.
test/normalize_ops_test.cpp Add tests for symbolic normalization behavior (notably dyn_slice bounds).
test/normalize_attributes_test.cpp New tests that characterize normalize_attributes integer normalization surface.
test/gpu/eval_expr_from_shape_lowering.cpp New GPU lowering tests for keeping slice metadata on host and copying results when needed.
test/gpu/dyn_slice_lowering.cpp New GPU lowering tests for dyn_slice runtime metadata movement/synchronization.
src/targets/gpu/lowering.cpp Extend lowering to support dyn_slice and host-only eval_expr_from_shape metadata usage.
src/sym.cpp Add interval::contains, fixed_value, provable_equal, folding helpers, and bare-number expr deserialization.
src/onnx/parse_where.cpp Simplify Where parsing and rely on add_common_op for broadcasting.
src/onnx/parse_trilu.cpp Implement symbolic-path Trilu parsing using dynamic-range + broadcasted mask construction.
src/onnx/parse_squeeze.cpp Update copyright year.
src/onnx/parse_slice.cpp Convert certain symbolic Slice cases into dyn_slice using sym_eval of bounds.
src/onnx/parse_shape.cpp Refactor start/end normalization into a helper and tighten types.
src/onnx/parse_reshape.cpp Use sym_eval to resolve reshape dims symbolically when possible (via eval_expr_from_shape).
src/onnx/parse_range.cpp When possible, convert symbolic Range into dynamic_range with symbolic output dimension.
src/onnx/parse_generic_op.cpp Add includes needed for new generic parsing behavior.
src/onnx/parse_expand.cpp Use sym_eval to compute symbolic broadcast output dims and pass them to broadcast_with_dims.
src/onnx/parse_constant_of_shape.cpp Use sym_eval to allocate with a symbolic output shape when possible.
src/onnx/parse_compare_op.cpp Update copyright year.
src/onnx/parse_cast.cpp Minor refactor + include fix.
src/onnx/include/migraphx/onnx/op_parser.hpp Update copyright year; remove unused include.
src/onnx/include/migraphx/onnx/onnx_parser.hpp Add missing includes (optional, limits, unordered_set) used by parser implementation.
src/normalize_attributes.cpp Add symbolic attribute normalization for sym::expr-backed attributes and refactor integer path.
src/instruction.cpp Implement instruction::sym_eval() with memoization and symbolic_compute fallback.
src/include/migraphx/tensor_view.hpp Fix bounds assertions and make to_vector() usable for const T views.
src/include/migraphx/sym.hpp Expose interval::contains, provable_equal, fixed_value, and fold_min/fold_max.
src/include/migraphx/sym_argument.hpp New public header defining sym_argument and helpers for symbolic tensor evaluation.
src/include/migraphx/operators.hpp Register dyn_slice operator header.
src/include/migraphx/operation.hpp Add symbolic_compute to the public operation interface and type-erased dispatch.
src/include/migraphx/op/where.hpp Add where::symbolic_compute to enable symbolic evaluation where condition is fixed.
src/include/migraphx/op/unsqueeze.hpp Add unsqueeze::symbolic_compute pass-through behavior.
src/include/migraphx/op/unary.hpp Add optional symbolic compute path to unary ops via CRTP flag.
src/include/migraphx/op/sub.hpp Enable symbolic compute for sub.
src/include/migraphx/op/squeeze.hpp Add squeeze::symbolic_compute pass-through behavior.
src/include/migraphx/op/slice.hpp Clarify symbolic-shape constraints and add slice::symbolic_compute for 1D symbolic tensors.
src/include/migraphx/op/reshape.hpp Add reshape::symbolic_compute pass-through behavior.
src/include/migraphx/op/normalize_attribute.hpp Document symbolic normalization behavior for expression attributes.
src/include/migraphx/op/multibroadcast.hpp Add multibroadcast::symbolic_compute broadcast behavior.
src/include/migraphx/op/mul.hpp Enable symbolic compute for mul.
src/include/migraphx/op/identity.hpp Add identity::symbolic_compute pass-through behavior.
src/include/migraphx/op/gather.hpp Add limited gather::symbolic_compute (axis 0, fixed indices).
src/include/migraphx/op/equal.hpp Enable symbolic compute for equal using provable_equal.
src/include/migraphx/op/dynamic_range.hpp Add optional symbolic output_dim to produce symbolic output shape for Range.
src/include/migraphx/op/dyn_slice.hpp New dyn_slice operator with symbolic-bound attributes + runtime-bound inputs.
src/include/migraphx/op/div.hpp Enable symbolic compute for div with conservative div-by-zero rejection.
src/include/migraphx/op/dimensions_of.hpp Add dimensions_of::symbolic_compute to return symbolic dimension expressions.
src/include/migraphx/op/convert.hpp Enable symbolic compute for safe int64bool conversions.
src/include/migraphx/op/concat.hpp Add limited concat::symbolic_compute for axis-0 concatenation of symbolic tensors.
src/include/migraphx/op/broadcast.hpp Add broadcast::symbolic_compute broadcast behavior.
src/include/migraphx/op/broadcast_with_dims.hpp Add optional out_dyn_dims attribute and validation for fully symbolic broadcast outputs.
src/include/migraphx/op/binary.hpp Add optional symbolic compute path to binary ops via CRTP flag.
src/include/migraphx/op/add.hpp Enable symbolic compute for add.
src/include/migraphx/instruction.hpp Add instruction::sym_eval() declaration.
src/include/migraphx/dim_like.hpp Add helpers for dimension lists (all_ints, to_ints) and include updates.
src/CMakeLists.txt Register dyn_slice operator for build.
CHANGELOG.md Document new dyn_slice operator and symbolic attribute normalization; clarify slice symbolic support.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/sym.cpp
Comment on lines +204 to 207
bool interval::contains(const scalar& value) const
{
return not scalar_less(value, min) and not scalar_less(max, value);
}
Comment thread src/onnx/parse_where.cpp Outdated
Comment thread src/include/migraphx/op/broadcast.hpp Outdated
Comment thread src/include/migraphx/sym_argument.hpp Outdated
Comment thread src/include/migraphx/sym_argument.hpp Outdated
Comment thread src/instruction.cpp Outdated
Comment thread src/include/migraphx/sym_argument.hpp Outdated
Comment thread src/include/migraphx/sym_argument.hpp Outdated
Comment thread src/include/migraphx/op/equal.hpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 70 out of 73 changed files in this pull request and generated no new comments.

Comment thread src/include/migraphx/sym_argument.hpp Outdated
@gh-app-migraphx-bot-pr-write

gh-app-migraphx-bot-pr-write Bot commented Aug 23, 2026

Copy link
Copy Markdown
Test Batch New Rate (938454) Old Rate (3a503c)* Diff Status
torchvision-resnet50 64 3,298.68 3,264.92 1.03%
torchvision-resnet50_fp16 64 7,882.54 7,548.67 4.42%
torchvision-densenet121 32 2,491.57 2,483.99 0.31%
torchvision-densenet121_fp16 32 5,017.44 5,004.24 0.26%
torchvision-inceptionv3 32 2,058.98 2,058.51 0.02%
torchvision-inceptionv3_fp16 32 4,436.83 4,416.99 0.45%
cadene-inceptionv4 16 819.06 820.61 -0.19%
cadene-resnext64x4 16 785.60 782.78 0.36%
slim-mobilenet 64 8,367.54 8,386.36 -0.22%
slim-nasnetalarge 64 229.25 228.86 0.17%
slim-resnet50v2 64 3,229.00 3,180.91 1.51%
bert-mrpc-onnx 8 1,170.52 1,168.84 0.14%
bert-mrpc-tf 1 504.31 498.63 1.14%
pytorch-examples-wlang-gru 1 376.07 473.35 -20.55% 🔴
pytorch-examples-wlang-lstm 1 547.79 384.83 42.35% 🔆
torchvision-resnet50_1 1 1,050.19 1,046.63 0.34%
cadene-dpn92_1 1 454.84 437.32 4.01%
cadene-resnext101_1 1 364.38 365.89 -0.41%
onnx-taau-downsample 1 841.29 844.09 -0.33%
dlrm-criteoterabyte 1 32.27 32.42 -0.46%
dlrm-criteoterabyte_fp16 1 51.54 51.80 -0.51%
agentmodel 1 15,064.55 9,209.12 63.58% 🔆
unet_fp16 2 58.29 58.80 -0.86%
resnet50v1_fp16 1 1,449.97 1,366.11 6.14% 🔆
resnet50v1_int8 1 1,841.86 1,883.96 -2.23%
bert_base_cased_fp16 64 1,100.24 1,098.16 0.19%
bert_large_uncased_fp16 32 347.22 345.59 0.47%
bert_large_fp16 1 207.33 206.59 0.36%
distilgpt2_fp16 16 2,092.73 2,092.89 -0.01%
yolov5s 1 563.24 558.33 0.88%
tinyllama 1 45.80 45.83 -0.07%
vicuna-fastchat 1 44.25 44.20 0.12%
whisper-tiny-encoder 1 413.38 411.87 0.37%
whisper-tiny-decoder 1 409.18 408.48 0.17%
llama2_7b 1 20.87 20.84 0.14%
qwen1.5-7b 1 23.69 23.58 0.46%
phi3-3.8b 1 28.36 26.72 6.16% 🔆
llama3-8b 1 22.76 21.80 4.40%
whisper-large-encoder 1 10.17 10.18 -0.08%
whisper-large-decoder 1 107.56 105.30 2.15%
mistral-7b 1 23.82 23.78 0.20%
FLUX.1-schnell 1 789.70 755.22 4.57%

Regressions detected 🔴

* No develop baseline was found for this PR's branch point; compared against the latest available develop run instead.

@gh-app-migraphx-bot-pr-write

Copy link
Copy Markdown
Test Status Result
bert-mrpc-onnx PASSED: MIGraphX meets tolerance
bert-mrpc-tf ERROR - check error output
traceback
Traceback (most recent call last):
File "/src/AMDMIGraphX/tools/accuracy/accuracy_checker.py", line 377, in
main()
File "/src/AMDMIGraphX/tools/accuracy/accuracy_checker.py", line 313, in main
import tensorflow as tf
File "/usr/local/lib/python3.12/dist-packages/tensorflow/init.py", line 40, in
from tensorflow.python import pywrap_tensorflow as _pywrap_tensorflow # pylint: disable=unused-import
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.12/dist-packages/tensorflow/python/pywrap_tensorflow.py", line 37, in
self_check.preload_check()
File "/usr/local/lib/python3.12/dist-packages/tensorflow/python/platform/self_check.py", line 63, in preload_check
from tensorflow.python.platform import _pywrap_cpu_feature_guard
ImportError: libnuma.so.1: cannot open shared object file: No such file or directory
pytorch-examples-wlang-gru 🔴 FAILED: MIGraphX is not within tolerance - check verbose output
pytorch-examples-wlang-lstm 🔴 FAILED: MIGraphX is not within tolerance - check verbose output
dlrm-criteoterabyte PASSED: MIGraphX meets tolerance
agentmodel PASSED: MIGraphX meets tolerance
unet PASSED: MIGraphX meets tolerance
resnet50v1 PASSED: MIGraphX meets tolerance
bert_base_cased_fp16 PASSED: MIGraphX meets tolerance
bert_large_uncased_fp16 🔴 FAILED: MIGraphX is not within tolerance - check verbose output
bert_large PASSED: MIGraphX meets tolerance
yolov5s PASSED: MIGraphX meets tolerance
tinyllama PASSED: MIGraphX meets tolerance
vicuna-fastchat PASSED: MIGraphX meets tolerance
whisper-tiny-encoder PASSED: MIGraphX meets tolerance
whisper-tiny-decoder PASSED: MIGraphX meets tolerance
distilgpt2_fp16 🔴 FAILED: MIGraphX is not within tolerance - check verbose output
llama2_7b PASSED: MIGraphX meets tolerance
qwen1.5-7b PASSED: MIGraphX meets tolerance
phi3-3.8b PASSED: MIGraphX meets tolerance
llama3-8b PASSED: MIGraphX meets tolerance
whisper-large-encoder PASSED: MIGraphX meets tolerance
whisper-large-decoder PASSED: MIGraphX meets tolerance
mistral-7b PASSED: MIGraphX meets tolerance
FLUX.1-schnell PASSED: MIGraphX meets tolerance

Comment on lines +42 to +43
static constexpr bool enable_symbolic_compute = false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? Just for ops that inherit another op?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, not every derived op can support symbolic compute

Comment thread src/include/migraphx/op/concat.hpp Outdated
sym_argument symbolic_compute(const shape& output_shape,
const std::vector<sym_argument>& args) const
{
if(axis != 0 or args.empty() or any_of(args, [](const auto& arg) { return arg.empty(); }))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does axis have to be 0? Shouldn't the condition be instead that the args are 1 dimensional?

sym_argument symbolic_compute(const shape& output_shape,
const std::vector<sym_argument>& args) const
{
if(args.size() != 1 or

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there being 1 argument already be handled by compute_shape()?

return [](auto x, auto y) {
if constexpr(std::is_same<std::decay_t<decltype(y)>, sym::expr>{})
{
if(y.eval_interval().contains(0))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for handling an empty interval?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no its handling the case where there could be a division by 0

if(args.empty() or args[0].get_shape().dynamic() or args[0].get_shape().ndim() != 1 or
args[0].empty())
return {};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for handling slice after something like dimensions_of, right? It's somewhat unfortunate that all this handling code for normalizing the attributes needs to be redone here again.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it uses the same normalization code but i dont see how we can avoid having to call normalization altogether

Comment thread src/sym.cpp
return std::nullopt;
}

std::optional<scalar> fixed_value(const expr& expression)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to check that an expression has a single possible value using the interval analysis? If so, please add a short explanation comment for the function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya, adding comment

Comment thread src/instruction.cpp
return result;
}

sym_argument instruction::sym_eval() const

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a short comment for this function.

Comment thread src/onnx/parse_shape.cpp Outdated
{
std::vector<op_desc> operators() const { return {{"Shape"}}; }

static std::pair<std::size_t, std::size_t>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a functional change here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no Im going to revert this. There was previously a functional change which is why I rewrote it, but thats not necessary now.


#include <onnx_test.hpp>

TEST_CASE(symbolic_reshape_markers)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this trying to test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for when the reshape dims contain 0 and -1. But this is messy, I am rewriting it as separate and more focused tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants