Skip to content

Rename getRawLoad to getRawAsync#4333

Merged
rdspring1 merged 1 commit intomainfrom
rename_load_to_async
Apr 30, 2025
Merged

Rename getRawLoad to getRawAsync#4333
rdspring1 merged 1 commit intomainfrom
rename_load_to_async

Conversation

@rdspring1
Copy link
Collaborator

@rdspring1 rdspring1 commented Apr 29, 2025

This PR renames getRawLoad to getRawAsync in ParallelDimensionMap. Recently, CircularBufferLoopStage changed LoadWarp to AsyncWarp. This function corresponds with LoadWarp and needs to be updated accordingly.

@github-actions
Copy link

github-actions bot commented Apr 29, 2025

Review updated until commit e937d1f

Description

  • Rename getRawLoad to getRawAsync in ParallelDimensionMap

  • Update method calls in CudaKernelGenerator

  • Update method documentation in parallel_dimension_map.h


Changes walkthrough 📝

Relevant files
Enhancement
codegen.cpp
Update `getRawLoad` to `getRawAsync`                                         

csrc/codegen.cpp

  • Updated getRawLoad to getRawAsync in genLoadBlockDim method
+3/-3     
parallel_dimension_map.cpp
Rename `getRawLoad` to `getRawAsync`                                         

csrc/parallel_dimension_map.cpp

  • Renamed getRawLoad to getRawAsync
+1/-1     
parallel_dimension_map.h
Rename `getRawLoad` to `getRawAsync` and update documentation

csrc/parallel_dimension_map.h

  • Renamed getRawLoad to getRawAsync
  • Updated method documentation to reflect AsyncWarp usage
  • +4/-4     

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review

    Incorrect Implementation

    The implementation of getRawAsync seems to be incorrect. It returns a padded value when warp specialization is enabled, which is different from the behavior of the old getRawLoad method. This change might lead to incorrect behavior in the code.

    Val* ParallelDimensionMap::getRawAsync(ParallelType pt) const {
      if (warp_specialized_types_.count(pt)) {
        return IrBuilder::create<Val>(
            getWarpSpecializationPaddedVal(pt), DataType::Index);
      }
    Documentation Mismatch

    The documentation for getRawAsync does not match its implementation. The comment suggests that the method returns a padded value for warp specialization, but the implementation returns a padded value regardless of warp specialization.

    //! of without warp specialization, this is the same as getRaw(pt). For warp
    //! warp specialization on pt, the result is padded_value. The last part of
    //! pt is used for AsyncWarp warp group.

    @rdspring1 rdspring1 force-pushed the rename_load_to_async branch from af66969 to e937d1f Compare April 30, 2025 01:31
    @rdspring1 rdspring1 changed the base branch from select_warp_predicate to main April 30, 2025 01:31
    @rdspring1
    Copy link
    Collaborator Author

    !test

    @rdspring1 rdspring1 merged commit 8d94409 into main Apr 30, 2025
    52 of 53 checks passed
    @rdspring1 rdspring1 deleted the rename_load_to_async branch April 30, 2025 17:14
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants