Skip to content

[WIP] Avoid redundancy with resize ops#3713

Draft
naoyam wants to merge 1 commit intomainfrom
resize_predicate
Draft

[WIP] Avoid redundancy with resize ops#3713
naoyam wants to merge 1 commit intomainfrom
resize_predicate

Conversation

@naoyam
Copy link
Collaborator

@naoyam naoyam commented Jan 15, 2025

Closes #3710

@naoyam
Copy link
Collaborator Author

naoyam commented Jan 15, 2025

!test

@github-actions
Copy link

github-actions bot commented Jan 15, 2025

PR Reviewer Guide 🔍

(Review updated until commit 9b7cda6)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
⚡ Recommended focus areas for review

Logic Change

The function getPredicates has been modified to predicate the input ID of a resize operation. This change may have unintended consequences and should be thoroughly reviewed.

std::vector<IterDomain*> predicate_domains = getPredicateDomains(tv, expr);

const IndexingInfo& index_info =
    computeIndex(expr, predicate_domains, for_loops);

const auto& index_map = index_info.index_map;

const std::unordered_map<Val*, Val*> replacement_map_start =
    getPredicateIndexReplacementMap(
        tv,
        for_loops,
        index_map,
        traversalGraph(),
        index_info.traversal_path,
        id_model_,
        /*is_start_predicate=*/true,
        /*unswitched_loop=*/unswitched_loop);

const std::unordered_map<Val*, Val*> replacement_map_stop =
    getPredicateIndexReplacementMap(
        tv,
        for_loops,
        index_map,
        traversalGraph(),
        index_info.traversal_path,
        id_model_,
        /*is_start_predicate=*/false,
        /*unswitched_loop=*/unswitched_loop);

// When resize is involved, predicate its input ID as well to avoid
// redudancy. This is only necessary if a predicated resize is
// preceded by a split, however, for now it's always predicated
// with an exception of static resize. See
// PredicateIndexingTest.SplitThenPad for a concrete example.
for (const auto& [eg, direction] : index_info.traversal_path) {
  auto resize = dynamic_cast<Resize*>(eg->front());
  if (resize == nullptr) {
    continue;
  }

  // TODO: It seems this shouldn't be predicated when the direction is
  // Forward, i.e., when resize ops are propagated from
  // producers to consumers. For example, ResizeTest.SliceThenPadLeftHalf
  // would fail with this. Revisit for the Forward case if necessary.
  if (direction == Direction::Forward) {
    continue;
  }

  // If the input ID is guaranteed to cover the output ID, then
  // the input index should never exceed its boundary.
  if (resize->leftExpand()->isConstInt() &&
      resize->rightExpand()->isConstInt()) {
    auto left_int = resize->leftExpand()->evaluate().as<int64_t>();
    auto right_int = resize->rightExpand()->evaluate().as<int64_t>();
    // If the traversal direction is forward, the predicate is not
    // necessary if both of the left and right factors are
    // non-negative as the ouput ID is guaranteed to cover the
    // input ID. Similarly, if it's backward, it is not necessary
    // if they are non-positive.
    if ((direction == Direction::Forward && left_int >= 0 &&
         right_int >= 0) ||
        (direction == Direction::Backward && left_int <= 0 &&
         right_int <= 0)) {
      continue;
    }
  }

  IterDomain* id_to_predicate =
      direction == Direction::Forward ? resize->out() : resize->in();

  predicate_domains.push_back(id_to_predicate);
}
Potential Bug

The TODO comment on line 1107 suggests that the current implementation may not be correct for the Forward direction. This should be investigated and fixed.

// Forward, i.e., when resize ops are propagated from
// producers to consumers. For example, ResizeTest.SliceThenPadLeftHalf
// would fail with this. Revisit for the Forward case if necessary.
if (direction == Direction::Forward) {
  continue;
}

@naoyam
Copy link
Collaborator Author

naoyam commented Jan 17, 2025

!test

@naoyam naoyam changed the title Avoid redundancy with resize ops [WIP] Avoid redundancy with resize ops Apr 4, 2025
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.

Redundant accesses due to missing predicates in resize

1 participant