Skip to content

Conversation

@naoyam
Copy link
Collaborator

@naoyam naoyam commented Dec 19, 2025

This PR introduces the combine operation as discussed in the RaggedIterDomain design doc.

One design decision that I changed from the original design doc is about detecting and validating component iter domains. Previously, I was thinking about using the exact graph to find the corresponding component iter domain for a given ragged iter domain (e.g., #5550 (comment)). However, it won't work, for example, when a fusion is segmented and a segment does not have the corresponding Partition expr for a RaggedIterDomain. For example, when a tensor is used as an input for asNested, followed by some other operations, if the fusion is segmented after some operations, the latter segment won't be able to see the asNested and the Partition operations as they don't exist in the segment. This could be alleviated by providing an exact graph for the whole complete fusion, but more fundamentally, if a fusion has a nested tensor as an input, there doesn't seem to be any reasonable way to attach a Partition expr.

See doc/dev/ragged_iter_domain_combine_design_doc.md‎ for detailed discussions. At this moment, I decided to not worry too much about the validation and assume the correctness is guaranteed by the user.

Note that partitioning is still limited to 1D extents. Multi-dim offsets will be the next step of this series of RPs.

@naoyam
Copy link
Collaborator Author

naoyam commented Dec 19, 2025

!test

@github-actions
Copy link

github-actions bot commented Dec 19, 2025

Review updated until commit 3a80926

Description

  • Implement RaggedIterDomain::combine() method to flatten ragged structures back to regular IterDomains

  • Add Combine IR expression class to represent component + ragged -> combined operation

  • Include comprehensive validation with best-effort component-ragged pairing verification

  • Add extensive test coverage for combine functionality, validation scenarios, and nested tensor operations

  • Provide detailed design document explaining validation strategy and architectural decisions

Changes walkthrough

Relevant files
Enhancement
internal_base_nodes.cpp
Implement combine method for RaggedIterDomain                       

csrc/ir/internal_base_nodes.cpp

  • Implement RaggedIterDomain::combine() method with input validation and
    symbolic extent creation
  • Add Combine expression creation linking component + ragged -> combined
    IterDomain
  • Include validation logic that checks Partition definitions when
    available, otherwise trusts user
  • +100/-0 
    internal_nodes.cpp
    Add Combine IR expression class                                                   

    csrc/ir/internal_nodes.cpp

  • Add Combine class definition with component and ragged inputs and
    combined output
  • Implement toString() and toInlineString() methods for Combine
    expression
  • Add NVFUSER_DEFINE_CLONE_AND_CREATE for Combine class
  • +27/-0   
    dispatch.h
    Add Combine to expression dispatch                                             

    csrc/dispatch.h

    • Add Combine to the expression dispatch macro list
    +1/-0     
    internal_base_nodes.h
    Add combine method declaration to RaggedIterDomain             

    csrc/ir/internal_base_nodes.h

  • Add combine() method declaration to RaggedIterDomain class
  • Include detailed documentation explaining the operation and usage
  • +16/-0   
    internal_nodes.h
    Add Combine class declaration                                                       

    csrc/ir/internal_nodes.h

  • Add Combine class declaration with out(), component(), and ragged()
    accessors
  • Include comprehensive documentation for the Combine expression
  • +38/-0   
    Tests
    test_ragged_iter_domain.cpp
    Add comprehensive test coverage for combine functionality

    tests/cpp/test_ragged_iter_domain.cpp

  • Add CombineBasic test for basic component + ragged -> combined
    functionality
  • Add validation tests for null inputs, wrong types, and
    component-ragged mismatches
  • Add AsNestedThenCombine test for nested tensor flattening
  • Add AsNestedThenSetThenCombine test for combine after propagation
    operations
  • Add AsNestedThenSetThenCombineInvalidComponent test for validation
    behavior
  • +236/-0 
    Documentation
    ragged_iter_domain_combine_design_doc.md
    Add design document for combine validation strategy           

    doc/dev/ragged_iter_domain_combine_design_doc.md

  • Provide comprehensive design document analyzing validation strategies
    for component-ragged pairing
  • Compare four design options and justify choosing Option 3 (validate
    when possible, trust user otherwise)
  • Explain architectural constraints and why complete validation is
    infeasible
  • Document implementation approach and future considerations
  • +354/-0 

    PR Reviewer Guide

    Here are some key observations to aid the review process:

    🧪 PR contains tests
    ⚡ Recommended focus areas for review
    Memory Management

    The combine method creates a symbolic extent Val and IterDomain using IrBuilder::createInContainer. Ensure proper memory management and that these created objects are properly tracked by the IR container to prevent memory leaks.

    Val* combined_extent =
        IrBuilder::createInContainer<Val>(container, DataType::Index);
    
    // Create the combined IterDomain with the symbolic extent
    IterDomain* combined_id = IterDomainBuilder(zero, combined_extent)
                                  .parallel_type(ParallelType::Serial)
                                  .iter_type(IterType::Iteration)
                                  .build();
    Validation Logic

    The validation logic checks for Partition definition and validates component matching. However, after set operations or in segmented fusions, the validation is skipped. Consider if additional runtime checks or assertions could help catch errors in these scenarios.

    // Validate component-ragged pairing when Partition definition is available
    // (Option 3 of doc/dev/ragged_iter_domain_combine_design_doc.md).
    // Only validate when the RaggedIterDomain has a direct Partition definition.
    // After propagation (e.g., set() operations), the definition may be nullptr,
    // in which case we trust the user to provide the correct component.
    if (ragged->definition() != nullptr &&
        ragged->definition()->isA<Partition>()) {
      auto* partition = ragged->definition()->as<Partition>();
      IterDomain* expected_component = partition->component();
    
      NVF_ERROR(
          component == expected_component,
          "combine: component mismatch. The provided component does not match ",
          "the component from the Partition that created this "
          "RaggedIterDomain.\n",
          "  Provided component: ",
          component->toString(),
          "\n",
          "  Expected component: ",
          expected_component->toString());
    }
    // If no Partition definition (after set, in segmented fusion, or external
    // input), trust the user and proceed without validation
    Test Coverage

    The tests cover basic functionality and validation scenarios well. Consider adding tests for edge cases like empty extents tensor, very large extents values, or multi-dimensional scenarios that might be added in future iterations.

    TEST_F(RaggedIterDomainTest, CombineBasic) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      // Create extents tensor
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      // Create a regular IterDomain to partition
      auto orig_id = IterDomainBuilder(
                         fusion.zeroVal(DataType::Index),
                         IrBuilder::create<Val>(325L, DataType::Index))
                         .build();
    
      // Partition into component and ragged
      auto [component_id, ragged_id] =
          RaggedIterDomain::partition(orig_id, extents);
    
      // Verify partition worked
      EXPECT_NE(component_id, nullptr);
      EXPECT_NE(ragged_id, nullptr);
      EXPECT_TRUE(component_id->isA<IterDomain>());
      EXPECT_TRUE(ragged_id->isA<RaggedIterDomain>());
    
      // Now combine them back
      auto combined_id = RaggedIterDomain::combine(component_id, ragged_id);
    
      // Verify combine worked
      EXPECT_NE(combined_id, nullptr);
      EXPECT_TRUE(combined_id->isA<IterDomain>());
      EXPECT_FALSE(combined_id->isA<RaggedIterDomain>());
    
      // Verify the combine has a definition (Combine expr)
      EXPECT_NE(combined_id->definition(), nullptr);
      EXPECT_TRUE(combined_id->definition()->isA<Combine>());
    
      // Verify the Combine expression has correct inputs
      auto combine_expr = combined_id->definition()->as<Combine>();
      EXPECT_EQ(combine_expr->component(), component_id);
      EXPECT_EQ(combine_expr->ragged(), ragged_id);
      EXPECT_EQ(combine_expr->out(), combined_id);
    }
    
    // Test combine validation: null component
    TEST_F(RaggedIterDomainTest, CombineValidationNullComponent) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      auto ragged_id =
          IrBuilder::create<RaggedIterDomain>(extents, IterType::Iteration);
    
      // Should fail with null component
      EXPECT_THROW(
          RaggedIterDomain::combine(nullptr, ragged_id), nvfuser::nvfError);
    }
    
    // Test combine validation: null ragged
    TEST_F(RaggedIterDomainTest, CombineValidationNullRagged) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto component_id = IterDomainBuilder(
                              fusion.zeroVal(DataType::Index),
                              IrBuilder::create<Val>(3L, DataType::Index))
                              .build();
    
      // Should fail with null ragged
      EXPECT_THROW(
          RaggedIterDomain::combine(component_id, nullptr), nvfuser::nvfError);
    }
    
    // Test combine validation: component is RaggedIterDomain
    TEST_F(RaggedIterDomainTest, CombineValidationComponentIsRagged) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto extents1 = makeSymbolicTensor(1, DataType::Index);
      auto extents2 = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents1);
      fusion.addInput(extents2);
    
      auto ragged_id1 =
          IrBuilder::create<RaggedIterDomain>(extents1, IterType::Iteration);
      auto ragged_id2 =
          IrBuilder::create<RaggedIterDomain>(extents2, IterType::Iteration);
    
      // Should fail when component is also RaggedIterDomain
      EXPECT_THROW(
          RaggedIterDomain::combine(ragged_id1, ragged_id2), nvfuser::nvfError);
    }
    
    // asNested basic functionality
    TEST_F(RaggedIterDomainTest, AsNestedBasic) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto data = makeSymbolicTensor(2, DataType::Float);
      fusion.addInput(data);
    
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      // Create nested tensor from dimension 0
      auto nested = asNested(data, extents, 0);
    
      fusion.addOutput(nested);
    
      // Verify the output is a new TensorView
      EXPECT_TRUE(nested != nullptr);
      EXPECT_NE(nested, data);
      EXPECT_TRUE(nested->isA<TensorView>());
    
      // Verify nested tensor has 3 dimensions: [component, ragged,
      // original_dim1]
      EXPECT_EQ(nested->nDims(), 3);
    
      // First axis should be a regular IterDomain (component)
      EXPECT_TRUE(nested->axis(0)->isStrictlyA<IterDomain>());
      EXPECT_FALSE(nested->axis(0)->isA<RaggedIterDomain>());
    
      // Second axis should be a RaggedIterDomain
      EXPECT_TRUE(nested->axis(1)->isA<RaggedIterDomain>());
    
      // Third axis should be the original second dimension
      EXPECT_TRUE(nested->axis(2)->isStrictlyA<IterDomain>());
    
      // Verify the definition exists (LoadStoreOp for aliasing)
      EXPECT_TRUE(nested->definition() != nullptr);
      EXPECT_TRUE(nested->definition()->isA<LoadStoreOp>());
    
      // Verify the component and ragged IterDomains have Partition as their
      // definition
      EXPECT_TRUE(nested->axis(0)->definition() != nullptr);
      EXPECT_TRUE(nested->axis(0)->definition()->isA<Partition>());
      EXPECT_EQ(nested->axis(0)->definition(), nested->axis(1)->definition());
    }
    
    // Test combining nested tensor back to normal tensor
    TEST_F(RaggedIterDomainTest, AsNestedThenCombine) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto data = makeSymbolicTensor(2, DataType::Float);
      fusion.addInput(data);
    
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      // Create nested tensor from dimension 0
      auto nested = asNested(data, extents, 0);
    
      // Verify nested tensor has 3 dimensions: [component, ragged, original_dim1]
      EXPECT_EQ(nested->nDims(), 3);
      EXPECT_TRUE(nested->axis(0)->isStrictlyA<IterDomain>());
      EXPECT_TRUE(nested->axis(1)->isA<RaggedIterDomain>());
    
      // Get the component and ragged IterDomains
      auto component_id = nested->axis(0);
      auto ragged_id = nested->axis(1)->as<RaggedIterDomain>();
    
      // Combine them back into a normal IterDomain
      auto combined_id = RaggedIterDomain::combine(component_id, ragged_id);
    
      // Verify the combined IterDomain is a regular IterDomain, not ragged
      EXPECT_NE(combined_id, nullptr);
      EXPECT_TRUE(combined_id->isStrictlyA<IterDomain>());
      EXPECT_FALSE(combined_id->isA<RaggedIterDomain>());
    
      // Verify the combined IterDomain has a Combine definition
      EXPECT_NE(combined_id->definition(), nullptr);
      EXPECT_TRUE(combined_id->definition()->isA<Combine>());
    
      // Verify the Combine expression has correct inputs
      auto combine_expr = combined_id->definition()->as<Combine>();
      EXPECT_EQ(combine_expr->component(), component_id);
      EXPECT_EQ(combine_expr->ragged(), ragged_id);
      EXPECT_EQ(combine_expr->out(), combined_id);
    
      // Verify that the component came from the same Partition as the ragged
      EXPECT_NE(component_id->definition(), nullptr);
      EXPECT_TRUE(component_id->definition()->isA<Partition>());
      EXPECT_EQ(component_id->definition(), ragged_id->definition());
    
      auto partition_expr = component_id->definition()->as<Partition>();
      EXPECT_EQ(partition_expr->component(), component_id);
      EXPECT_EQ(partition_expr->ragged(), ragged_id);
    }
    
    // Test combining nested tensor back to normal tensor after set operation
    TEST_F(RaggedIterDomainTest, AsNestedThenSetThenCombine) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto data = makeSymbolicTensor(2, DataType::Float);
      fusion.addInput(data);
    
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      // Create nested tensor from dimension 0
      auto nested = asNested(data, extents, 0);
    
      // Insert a set operation after asNested
      auto nested_copy = set(nested);
    
      // Verify nested_copy tensor has 3 dimensions: [component, ragged,
      // original_dim1]
      EXPECT_EQ(nested_copy->nDims(), 3);
      EXPECT_TRUE(nested_copy->axis(0)->isStrictlyA<IterDomain>());
      EXPECT_TRUE(nested_copy->axis(1)->isA<RaggedIterDomain>());
    
      // Get the component and ragged IterDomains from the copy
      auto component_id = nested_copy->axis(0);
      auto ragged_id = nested_copy->axis(1)->as<RaggedIterDomain>();
    
      // Combine them back into a normal IterDomain. Even though
      // component_id and ragged_id are not directly produced by a
      // partition, this should succeed. See the next test
      // (AsNestedThenSetThenCombineInvalidComponent) for a failing example.
      auto combined_id = RaggedIterDomain::combine(component_id, ragged_id);
    
      // Verify the combined IterDomain is a regular IterDomain, not ragged
      EXPECT_NE(combined_id, nullptr);
      EXPECT_TRUE(combined_id->isStrictlyA<IterDomain>());
      EXPECT_FALSE(combined_id->isA<RaggedIterDomain>());
    
      // Verify the combined IterDomain has a Combine definition
      EXPECT_NE(combined_id->definition(), nullptr);
      EXPECT_TRUE(combined_id->definition()->isA<Combine>());
    
      // Verify the Combine expression has correct inputs
      auto combine_expr = combined_id->definition()->as<Combine>();
      EXPECT_EQ(combine_expr->component(), component_id);
      EXPECT_EQ(combine_expr->ragged(), ragged_id);
      EXPECT_EQ(combine_expr->out(), combined_id);
    }
    
    // Test combining with invalid component (not from same partition) - should
    // Test combining after set operation with invalid component
    // With Option 3 validation strategy, this does NOT throw an error
    // because after set(), the RaggedIterDomain loses its Partition definition
    // and validation is skipped (trusts the user)
    TEST_F(RaggedIterDomainTest, AsNestedThenSetThenCombineInvalidComponent) {
      Fusion fusion;
      FusionGuard fg(&fusion);
    
      auto data = makeSymbolicTensor(2, DataType::Float);
      fusion.addInput(data);
    
      auto extents = makeSymbolicTensor(1, DataType::Index);
      fusion.addInput(extents);
    
      // Create nested tensor from dimension 0
      auto nested = asNested(data, extents, 0);
    
      // Insert a set operation after asNested
      auto nested_copy = set(nested);
    
      // Verify nested_copy tensor has 3 dimensions: [component, ragged,
      // original_dim1]
      EXPECT_EQ(nested_copy->nDims(), 3);
      EXPECT_TRUE(nested_copy->axis(0)->isStrictlyA<IterDomain>());
      EXPECT_TRUE(nested_copy->axis(1)->isA<RaggedIterDomain>());
    
      // Get the ragged IterDomain from the copy
      auto ragged_id = nested_copy->axis(1)->as<RaggedIterDomain>();
    
      // Use an INVALID component: the third axis instead of the first
      // This is NOT the component from the partition, it's the original second
      // dimension
      auto invalid_component_id = nested_copy->axis(2);
    
      // With Option 3: After set(), the RaggedIterDomain no longer has a
      // Partition definition, so validation is skipped and the operation succeeds.
      // The user is responsible for providing the correct component.
      EXPECT_NO_THROW(RaggedIterDomain::combine(invalid_component_id, ragged_id));
    }

    @greptile-apps
    Copy link
    Contributor

    greptile-apps bot commented Dec 19, 2025

    Greptile Summary

    Implements the combine operation for RaggedIterDomain, which is the inverse of partition - merging component and ragged dimensions back into a single flattened IterDomain.

    Key changes:

    • Added RaggedIterDomain::combine() static method that creates a regular IterDomain with extent representing sum(extents)
    • Introduced new Combine expression type following the same pattern as Partition, Merge, and Split
    • Implemented "Option 3" validation strategy: validates component-ragged pairing when Partition definition exists, otherwise trusts the user (necessary for segmented fusions and propagated domains)
    • Comprehensive test coverage including validation tests, propagation through set(), and edge cases
    • Well-documented design document explaining architectural trade-offs

    Implementation note:
    The combined extent is created as a symbolic Val without an explicit sum(extents_tv, {0}) expression. This follows the comment in the code stating "we could use the sum output as the extent but we would need to extract the scalar value out of the 0-dim tensor." The extent relationship to the sum is left implicit, which may require special handling during lowering/indexing passes.

    Confidence Score: 4/5

    • Safe to merge with minor note about symbolic extent handling during lowering
    • Well-designed implementation with thorough validation, comprehensive tests, and clear documentation. Score is 4 (not 5) because the symbolic extent approach leaves the relationship to sum(extents) implicit, which may need verification during lowering/indexing passes to ensure the extent is correctly resolved
    • Pay attention to csrc/ir/internal_base_nodes.cpp - verify that symbolic extent handling works correctly in lowering passes

    Important Files Changed

    Filename Overview
    csrc/ir/internal_nodes.h Added Combine expression class definition following consistent pattern with Partition - clean design
    csrc/ir/internal_nodes.cpp Implemented Combine expression constructor and string methods - simple and follows existing patterns
    tests/cpp/test_ragged_iter_domain.cpp Added comprehensive tests for combine operation including validation, propagation through set(), and edge cases - thorough coverage
    csrc/ir/internal_base_nodes.cpp Implemented combine operation with symbolic extent and Option 3 validation strategy. Creates symbolic Val for extent without explicit sum expression - extent relationship left implicit

    Sequence Diagram

    sequenceDiagram
        participant User
        participant RaggedIterDomain
        participant IterDomainBuilder
        participant Combine as Combine Expr
        participant IrContainer
    
        User->>RaggedIterDomain: combine(component, ragged)
        
        RaggedIterDomain->>RaggedIterDomain: Validate inputs (null checks, types)
        RaggedIterDomain->>RaggedIterDomain: Check parallelization (must be Serial)
        RaggedIterDomain->>RaggedIterDomain: Check iter types (must be Iteration)
        
        alt ragged has Partition definition
            RaggedIterDomain->>Combine: Validate component matches partition
            Note over RaggedIterDomain,Combine: Option 3: Validate when possible
        else No Partition definition
            Note over RaggedIterDomain: Trust user (after set/segmentation)
        end
        
        RaggedIterDomain->>RaggedIterDomain: Get extents from ragged
        RaggedIterDomain->>RaggedIterDomain: Verify extents is 1D tensor
        
        RaggedIterDomain->>IrContainer: Create symbolic Val for combined_extent
        Note over RaggedIterDomain,IrContainer: Extent represents sum(extents)<br/>but not explicitly computed
        
        RaggedIterDomain->>IterDomainBuilder: Build output IterDomain
        IterDomainBuilder-->>RaggedIterDomain: combined_id (regular IterDomain)
        
        RaggedIterDomain->>Combine: Create Combine expression
        Combine->>Combine: addOutput(combined_id)
        Combine->>Combine: addInput(component)
        Combine->>Combine: addInput(ragged)
        
        RaggedIterDomain-->>User: Return combined_id
    
    Loading

    Copy link
    Contributor

    @greptile-apps greptile-apps bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    5 files reviewed, 2 comments

    Edit Code Review Agent Settings | Greptile

    Comment on lines +1060 to +1095
    NVF_ERROR(component != nullptr, "combine: component IterDomain is null");
    NVF_ERROR(ragged != nullptr, "combine: ragged IterDomain is null");

    NVF_ERROR(
    !component->isA<RaggedIterDomain>(),
    "combine: component must be a regular IterDomain, got RaggedIterDomain: ",
    component->toString());

    // Validate that component and ragged have compatible properties
    NVF_ERROR_EQ(
    component->getParallelType(),
    ParallelType::Serial,
    "Combining parallelized IterDomain not supported: ",
    component->toString());

    NVF_ERROR_EQ(
    ragged->getParallelType(),
    ParallelType::Serial,
    "Combining parallelized RaggedIterDomain not supported: ",
    ragged->toString());

    NVF_ERROR_EQ(
    component->getIterType(),
    IterType::Iteration,
    "combine: only IterType::Iteration is supported for component, got ",
    component->getIterType(),
    " for IterDomain: ",
    component->toString());

    NVF_ERROR_EQ(
    ragged->getIterType(),
    IterType::Iteration,
    "combine: only IterType::Iteration is supported for ragged, got ",
    ragged->getIterType(),
    " for RaggedIterDomain: ",
    ragged->toString());
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    style: Missing validation that component and ragged are semantically compatible or from the same partition. Consider verifying:

    • That the component extent matches the ragged extents dimension (component extent == extents_tv logical domain size)
    • Or that both came from the same Partition operation (component->definition() == ragged->definition())

    Without this, arbitrary unrelated IterDomains could be combined.

    Copy link
    Contributor

    @greptile-apps greptile-apps bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    6 files reviewed, 1 comment

    Edit Code Review Agent Settings | Greptile

    Comment on lines +1151 to +1152
    IrBuilder::createInContainer<Combine>(
    container, combined_id, component, ragged);
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    style: Unlike Partition which stores the extents tensor as an attribute (line 2634 in internal_nodes.cpp), Combine doesn't store it anywhere. While the extents can be accessed via ragged->extents(), this creates an inconsistency.

    If the ragged IterDomain later loses its connection to the original extents (e.g., through cloning or transformation), the Combine expression won't have a direct reference to them.

    Suggested change
    IrBuilder::createInContainer<Combine>(
    container, combined_id, component, ragged);
    // Store extents as an attribute for consistency with Partition
    IrBuilder::createInContainer<Combine>(
    container, combined_id, component, ragged, ragged->extents());

    Note: This would require updating the Combine class signature to accept and store extents as an attribute.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Copy link
    Contributor

    @greptile-apps greptile-apps bot left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    7 files reviewed, 1 comment

    Edit Code Review Agent Settings | Greptile

    Comment on lines +1141 to +1142
    Val* combined_extent =
    IrBuilder::createInContainer<Val>(container, DataType::Index);
    Copy link
    Contributor

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    style: The symbolic extent approach leaves the combined extent as an opaque Val without establishing its relationship to the actual sum. During lowering/indexing, the system needs to compute actual indices, but there's no expression connecting combined_extent to sum(extents_tv, {0}).

    Check whether this symbolic extent will cause issues during:

    • Index computation when iterating over the combined dimension
    • Extent analysis passes that need to know actual sizes
    • Fusion validation that checks dimension compatibility

    @naoyam naoyam changed the title [WIP] Combine (merge) for RaggedIterDomain Combine for RaggedIterDomain Dec 19, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants