Concretize all dynamic outputs of downstream expressions#420
Concretize all dynamic outputs of downstream expressions#420jacobhinkle wants to merge 28 commits intomainfrom
Conversation
|
This seemed to have fixed the immediate issue, but I am now hitting another issue with this repro where some input extents do not seem to be bound properly. This may be an entirely separate issue.. Concretized fusion: winds up with this expression evaluator: Here, |
|
I understand the second problem better now. With the concretization fix, the above The input to the second segment is I think we could address this by providing all input extents/scalars to every |
|
Pushed a change that propagates extent expressions P2C during concretization. This fixes cases like in #418 since the "proper" extents from the static reshape will get written downstream. Note that it may still be possible for those expressions to be used in another op (i.e. not an ID extent) using something like Note that there is a failing test: |
This reverts commit 9727388.
This is still failing due to segmentation not being able to infer output shapes.
|
The current state of this PR rewrites the concretized reshape so that its rfactor extents match those provided to the dynamic reshape op. This introduced some complication, and is failing when creating new I just tried an alternative: rolling this branch back to d675161 and cherry-picking bf9aa1e. In that approach, instead of making the reshape rfactor match the dynamic values, we instead propagate the |
|
!build |
|
Currently, there are two test failures caused by the same subtle issue. When we create a new However, the error comes in when we do an operation with some |
This reverts commit 6b85cf4.
|
!build |
| // NOTE: ops::newOutputTV would not necessarily be able to infer that the | ||
| // padded dimensions are all of the same size. However, we know that they are | ||
| // constructed such that that is the case, so we can use | ||
| auto out_domain = ops::newOutputDomain(resized_inputs); | ||
| // Override the concatenated dimension and insert an IterDomain with the true | ||
| // extent, if needed | ||
| if (!out_domain.at(cat_dim)->extent()->sameAs(concat_ext)) { | ||
| out_domain[cat_dim] = | ||
| IterDomainBuilder(out_domain.at(cat_dim)).extent(concat_ext).build(); | ||
| } | ||
| auto out = IrBuilder::create<TensorView>( | ||
| IrBuilder::create<TensorDomain>( | ||
| out_domain, TensorDomain::getContiguityFilledWith(out_domain, true)), | ||
| dtype); |
There was a problem hiding this comment.
This fixes a breakage that occurred when I made the other changes. It makes the output extent of cat look as it should (e.g. (i0 + i2) + i4) instead of creating a new symbolic extent as was done previously and which complicated concretization.
This is in lieu of replacing all uses of symbolic extents, which cannot be done reliably since they might appear as attributes or members of objects which are untracked. See #420
This change ensures that concretization visits all outputs to active expressions. Even though unused outputs do not directly affect the result of the Fusion, they still must not have Symbolic IterDomains during scheduling. Note that this pre-empts the more complicated #420. This fixes the immediate cause of #418. However, that test still fails due to another issue where scalars get lost during segmentation. The test included here only tests that concretization works properly; scheduling and execution will be tested in another PR.
|
Closing in favor of #591 |
1 participant
Fixes #418.
Previously, when concretizing a dynamic
Fusion, we find all statements between inputs and outputs and mutate everyVal. In the case ofExprs with multiple outputs, if some outputs are not used then they would not be concretized. This was the case in #418 in which thecountoutput fromWelfordwas not used, but it must be concretized. This PR:Valas well as the output of every intermediate expression between inputs and outputs.IterTypebut alsoextentexpressions from producer to consumer during concretizationfusion_ir_dynamicdebug dump option, which can be used to view the fusion before concretization.