Skip to content

Conversation

@zasdfgbnm
Copy link
Collaborator

No description provided.

@zasdfgbnm
Copy link
Collaborator Author

!test

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Description

  • Fixed contiguity computation for allocation domains

  • Improved handling of broadcast and reduction axes

  • Correctly traverse dimensions from right to left

  • Accurately compute stride dependencies for non-broadcast dims


Changes walkthrough 📝

Relevant files
Bug fix
fusion_cache_utils.cpp
Fix contiguity logic in allocation domain                               

csrc/runtime/fusion_cache_utils.cpp

  • Rewrote contiguity computation to traverse from right to left
  • Properly skip broadcast dimensions in stride calculation
  • Handle reduction dimensions with std::nullopt as intended
  • Use actual stride relationships to determine contiguity
  • +37/-10 

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review

    Possible Issue

    The logic for computing contiguity has been significantly rewritten, but the new implementation may incorrectly handle cases where broadcast dimensions are present, potentially leading to incorrect contiguity flags due to index misalignment between the allocation domain and sizes/strides arrays.

    const auto [sizes, strides] =
        inferAllocationSizesAndStrides(tensor, tv, ExpressionEvaluator());
    // Compute contiguity from right to left, based on allocation domain
    std::vector<std::optional<bool>> contiguity;
    auto allocation_domain = tv->getMaybeAllocationDomain();
    contiguity.resize(allocation_domain.size());
    
    int64_t index = sizes.size() - 1; // Start from rightmost in sizes/strides
    int64_t next_non_broadcast_shape = -1;
    int64_t next_non_broadcast_stride = -1;
    
    // Traverse allocation domain from right to left
    for (int64_t i = allocation_domain.size() - 1; i >= 0; i--) {
      auto id = allocation_domain[i];
    
      if (id->isReduction()) {
        // Reduction has no corresponding entry in sizes/strides
        contiguity[i] = std::nullopt;
      } else if (id->isBroadcast()) {
        // Broadcast has entry in sizes/strides, but we ignore it
        contiguity[i] = std::nullopt;
        index--; // Skip the corresponding entry in sizes/strides
      } else {
        // Non-reduction, non-broadcast dimension
        int64_t current_stride = strides[index];
        if (next_non_broadcast_shape == -1) {
          // This is the rightmost non-reduction non-broadcast dimension
          // Check if stride is 1 to determine contiguity
          contiguity[i] = (current_stride == 1);
        } else {
          // Check if current_stride == next_shape * next_stride
          // where next_shape/stride are from the next non-broadcast dimension
          contiguity[i] =
              (current_stride ==
               next_non_broadcast_shape * next_non_broadcast_stride);
        }
        // Update next_non_broadcast_shape and next_non_broadcast_stride
        next_non_broadcast_shape = sizes[index];
        next_non_broadcast_stride = strides[index];
        index--;
      }
    }
    tv->setContiguity(contiguity);
    Logic Error

    The loop traverses the allocation domain from right to left, but the index into sizes/strides is decremented unconditionally for non-broadcast non-reduction dimensions. This may result in incorrect pairing of dimensions if there are multiple broadcast dimensions, leading to wrong contiguity calculations.

    for (int64_t i = allocation_domain.size() - 1; i >= 0; i--) {
      auto id = allocation_domain[i];
    
      if (id->isReduction()) {
        // Reduction has no corresponding entry in sizes/strides
        contiguity[i] = std::nullopt;
      } else if (id->isBroadcast()) {
        // Broadcast has entry in sizes/strides, but we ignore it
        contiguity[i] = std::nullopt;
        index--; // Skip the corresponding entry in sizes/strides
      } else {
        // Non-reduction, non-broadcast dimension
        int64_t current_stride = strides[index];
        if (next_non_broadcast_shape == -1) {
          // This is the rightmost non-reduction non-broadcast dimension
          // Check if stride is 1 to determine contiguity
          contiguity[i] = (current_stride == 1);
        } else {
          // Check if current_stride == next_shape * next_stride
          // where next_shape/stride are from the next non-broadcast dimension
          contiguity[i] =
              (current_stride ==
               next_non_broadcast_shape * next_non_broadcast_stride);
        }
        // Update next_non_broadcast_shape and next_non_broadcast_stride
        next_non_broadcast_shape = sizes[index];
        next_non_broadcast_stride = strides[index];
        index--;
      }

    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.

    2 participants