Fix scheduling for fp16 reductions#370
Merged
jjsjann123 merged 9 commits into20_8_18_develfrom Sep 14, 2020
Merged
Conversation
jjsjann123
approved these changes
Sep 9, 2020
Collaborator
jjsjann123
left a comment
There was a problem hiding this comment.
LGTM. I'm not stamping it yet as I haven't got around to try the fix.
| // them. | ||
| struct FindOutputs : public IterVisitor { | ||
| const std::unordered_set<Val*>& of_; | ||
| std::unordered_set<Val*> outs; |
Collaborator
There was a problem hiding this comment.
nitpick, inconsistent name of of_ & outs;
| traverseFrom(fusion, fusion->outputs(), false); | ||
| }; | ||
|
|
||
| public: |
Collaborator
There was a problem hiding this comment.
nitpick, public in struct.
| // Use dependency check to find the reduction tv as it returns used values | ||
| // instead of exprs. | ||
| auto used_vals = DependencyCheck::getAllValsBetween( | ||
| {fusion.inputs().begin(), fusion.inputs().end()}, fusion.outputs()); |
Collaborator
There was a problem hiding this comment.
Does getAllValsBetween(...) have a smaller traverse space than fusion.exprs() because we might have trailing nodes after outputs? Just curious about the motivation of code change here.
| // tv->axis(-1)->parallelize(ParallelType::TIDx); | ||
| // } | ||
| } | ||
| TensorView* out0 = fusion->outputs()[0]->as<TensorView>(); |
Collaborator
There was a problem hiding this comment.
code below out0 should be cleaned as well. This is used in previously when scheduling is marked explicitly in transformation.
jjsjann123
added a commit
that referenced
this pull request
Sep 14, 2020
Fixes #357. Two things in this PR: Do type propagation even for profiling executor -> this is the root cause for bug reported in #357 Allow dtype argument in sum, which is simply handled in our type propagation. It exposes scheduling issue in #362, for which we added python tests (currently disabled). Fix is in PR #370, will enable after merge.
…ion didn't have their TIDX bound.
…ead binding issues
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
3 participants
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.
Fixes #362