Split symbolic dimension pass - #5123
Conversation
Regressions detected 🔴 * No develop baseline was found for this PR's branch point; compared against the latest available develop run instead. |
|
CharlieL7
left a comment
CharlieL7
left a comment
There was a problem hiding this comment.
I would like to see an explanation of the steps in the split_sym_dim pass. Especially what the collect_roots() , discover_blocks(), materialize(), coalesce() and specialize_blocks() functions are trying to do.
I also see that this is using broadcast and multibroadcast with symbolic output_dyn_dims but without 2 inputs. Which is why they then have to be materialized to using the out_lens attribute. Our original idea was to use a symbolic map to evaluate at runtime, but that plan changed.
I don't think the multibroadcast or broadcast with symbolic output_dyn_dim with only 1 input would be able to run compute(). So it's an op that has to be rewritten. That's the same issue as the TopK redesign I had. To avoid that, multibroadcast and broadcast should be handled like dyn_slice.
| unsupported_ops}; | ||
| } | ||
|
|
||
| static void materialize_returned_slice(module& m) |
There was a problem hiding this comment.
What is the purpose of this function?
| { | ||
| return { | ||
| enable_pass(disabled(MIGRAPHX_ENABLE_FULL_DYNAMIC{}), split_single_dyn_dim{}), | ||
| split_sym_dim{}, |
There was a problem hiding this comment.
I think this was the only place we use split_single_dyn_dim{}. So should probably just remove the pass rather than have dead code?
| "scatternd_none", | ||
| "select_module"}; | ||
| "select_module", | ||
| "slice"}; |
There was a problem hiding this comment.
Why add slice to this list?
| if(has_finalize(op)) | ||
| return false; |
There was a problem hiding this comment.
What was finalize() doing before these symbolic changes? As in, are there operators with a finalize() that can be evaluated?
| return root_ins.name() == "@literal" and root_ins.get_literal() == literal; | ||
| }); | ||
| auto new_lit = | ||
| existing == root_module->end() ? root_module->add_literal(literal) : existing; |
There was a problem hiding this comment.
Is this so we don't have a literal at the end of a module? Why is that an issue?
| unknown // no registered semantics -> forces a pass boundary | ||
| }; | ||
|
|
||
| // Sentinel the padded region must hold so a sensitive op's result is unchanged. |
There was a problem hiding this comment.
| // Sentinel the padded region must hold so a sensitive op's result is unchanged. | |
| // Sentinel value the padded region must hold so a sensitive op's result is unchanged. |
| // Sentinel the padded region must hold so a sensitive op's result is unchanged. | ||
| enum class fill_kind | ||
| { | ||
| none, // op is insensitive on this axis (parallel); fill is a don't-care |
There was a problem hiding this comment.
What does "(parallel)" mean?
| std::optional<fill_kind> reduce_identity(const std::string& name) | ||
| { | ||
| if(name == "reduce_max") | ||
| return fill_kind::lowest; | ||
| if(name == "reduce_min") | ||
| return fill_kind::highest; | ||
| if(name == "reduce_prod" or name == "reduce_all") | ||
| return fill_kind::one; | ||
| if(name == "reduce_sum" or name == "reduce_any") | ||
| return fill_kind::zero; | ||
| return std::nullopt; | ||
| } |
| std::vector<int64_t> reduce_axes(const operation& op, std::size_t ndim) | ||
| { | ||
| auto v = op.to_value(); | ||
| std::vector<int64_t> axes; | ||
| if(v.contains("axes")) | ||
| axes = v.at("axes").to_vector<int64_t>(); | ||
| int64_t rank = ndim; | ||
| for(auto& a : axes) | ||
| if(a < 0) | ||
| a += rank; | ||
| return axes; | ||
| } |
There was a problem hiding this comment.
This is redoing what normalize_attributes does because it's happening before the normalize_ops pass. Maybe we should just run normalize_ops before this pass?
| axis_policy axes = unsupported_policy(); | ||
| std::optional<symbolic_target_policy> symbolic_target; | ||
| }; | ||
|
|
There was a problem hiding this comment.
There are a lot of structs in this pass. Can you add descriptions how they are used?
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>
…nto split_sym_dim
2 participants
Motivation
Currently we only handle the dynamic batch case for using select modules, this creates the framework for generalizing for all axes.
Technical Details
The padding and slicing semantics are defined in terms of op-families where each family of ops (ie. convolution, pointwise, gemm, reduce, etc.) defines the characteristics of its input and output axes. These characteristics are then use to materialize pad-slice wrappings for each operator and then used to coalesce chains of operations that have compatible padding/masking characteristics.
See tests for examples of currently supported blocks.
WIP
Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot ApplicableFollow the LLVM AI Tool Use Policy for contributions using AI.