Always enable IdModel-based indexing when resize is used #3567
Conversation
| } else { | ||
| EnableOptionsGuard::getCurOptions().unset(EnableOption::IdModel); | ||
| } | ||
| EnableOptionsGuard::getCurOptions().set(EnableOption::IdModel, {"all"}); |
There was a problem hiding this comment.
I still keep these options here because "all" also enables loop generation using IdModel, which is required for inlining. I should probably also enable that in lower2device.cpp, but that'll be a future PR.
There was a problem hiding this comment.
Changes here are just not running the tests without IdModel since it doen't make sense anymore.
|
!test |
|
!test |
… enable_id_model_for_resize
|
!test |
1 similar comment
|
!test |
| auto exprs = DependencyCheck::getAllExprsBetween( | ||
| {tv->getLogicalDomain().begin(), tv->getLogicalDomain().end()}, | ||
| {tv->getMaybeAllocationDomain().begin(), |
There was a problem hiding this comment.
One issue found here
| @@ -845,14 +855,18 @@ std::vector<Val*> TensorIndexer::getIndexFor( | |||
| const auto& replacement_map = getIndexReplacementMap( | |||
| expr, as_consumer, info.loop_domains, for_loops, info.index_map); | |||
|
|
|||
| const auto index_groups = traversalGraph().toGroups(index_ids); | |||
| // Note that IDs of index_ids may be mapped as the traversal graph | |||
There was a problem hiding this comment.
Another issue found here. Previously, this function only returns a list of indices for a unique vector of ValGroups. This resulted in an error when the given index vector (index_ids) have mapped IDs.
| options.setProducerIndex(true); | ||
| options.setConsumerIndex(true); | ||
| options.setInlinePredicate(true); | ||
| options.setUnswitchPredicate(true); |
There was a problem hiding this comment.
Should we have options.setLoop(true) also?
There was a problem hiding this comment.
Nevermind, I see your comment in test_resize.cpp now.
Followup to #3567. I just found the Loop option is also necessary. With this option, the inlining analysis uses IdModel to understand if inlining is possible. The loop generation lowering pass also uses IdModel loop promotion to figure out which iter domains to use for each `ForLoop` node. The latter is not necessary for the resize scheduler at this moment, though.
I'm pretty much always assuming the new indexer is used when resize is involved, so enabling the new indexer automatically when resize is detected.
There are a couple of new issues found while running the tests.