Skip to content

Conversation

@Priya2698
Copy link
Collaborator

No description provided.

@github-actions
Copy link

github-actions bot commented Oct 16, 2025

Review updated until commit fa81b5e

Description

  • Fix stream condition logic in loop domain filtering

  • Remove redundant stream check in transform expr loop

  • Apply consistent stream parallelization condition

  • Simplify allocation domain transformation logic


Changes walkthrough 📝

Relevant files
Bug fix
finalize_multidevice_domains.cpp
Refactor stream and allocation domain logic                           

csrc/preseg_passes/finalize_multidevice_domains.cpp

  • Move stream parallelization condition into loop domain filter
  • Remove redundant stream check in transform expr processing
  • Use consistent shouldParallelizeAllocationOnStream condition
  • Simplify allocation-to-contiguity update logic
  • +3/-6     

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review

    Logic Change

    The filtering condition for stream dimensions has been moved from the loop iteration (post-split check) to the view definition (pre-split filter). This changes when shouldParallelizeAllocationOnStream(tv) is evaluated and may affect which expressions are processed. The impact on sharding correctness and allocation propagation should be validated, especially for cases where stream parallelization is disabled.

    tv->getLoopDomain() | std::views::filter([&tv](IterDomain* id) {
      return id->isDeviceDim() ||
          (id->isStream() && shouldParallelizeAllocationOnStream(tv));
    });

    @Priya2698
    Copy link
    Collaborator Author

    !test

    @Priya2698 Priya2698 marked this pull request as ready for review October 17, 2025 19:13
    @Priya2698 Priya2698 changed the title move condition Follow-up to #5353: Move another condition within views::filter Oct 17, 2025
    @Priya2698 Priya2698 requested a review from wujingyue October 17, 2025 19:15
    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