Skip to content

Remove HostIrLower::canLower as it's no longer used#4643

Merged
wujingyue merged 10 commits intomainfrom
wjy/canlower
Jun 17, 2025
Merged

Remove HostIrLower::canLower as it's no longer used#4643
wujingyue merged 10 commits intomainfrom
wjy/canlower

Conversation

@wujingyue
Copy link
Collaborator

@wujingyue wujingyue commented Jun 16, 2025

Follow-up to #4552 (comment)

@github-actions
Copy link

github-actions bot commented Jun 16, 2025

Review updated until commit 9b4eac3

Description

  • Remove unused HostIrLower::canLower function

  • Update tests to be more realistic

  • Improve test coverage by using MultiDeviceTest


Changes walkthrough 📝

Relevant files
Enhancement
lower.cpp
Remove unused function and update tests                                   

csrc/host_ir/lower.cpp

  • Removed HostIrLower::canLower function
  • Updated test cases to use MultiDeviceTest
  • +0/-38   
    lower.h
    Remove function declaration                                                           

    csrc/host_ir/lower.h

    • Removed declaration of HostIrLower::canLower
    +0/-5     

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🧪 No relevant tests
    ⚡ Recommended focus areas for review

    Possible Issue

    The removal of canLower might lead to issues in places where this function was previously used. Ensure that all calls to canLower have been appropriately handled or removed.

    namespace nvfuser {
    
    bool HostIrLower::isLowerableAsStandaloneHostOp(Expr* expr) {
    Documentation

    The comment about ignore_inner_resharding in the header file should be updated or removed to reflect the changes made in the PR.

    class HostIrLower {
     public:
      explicit HostIrLower(const HostIrLowerParams& params = HostIrLowerParams())

    @wujingyue wujingyue requested a review from Priya2698 June 16, 2025 06:47
    @wujingyue
    Copy link
    Collaborator Author

    !build

    Base automatically changed from wjy/insert to main June 16, 2025 22:35
    @wujingyue
    Copy link
    Collaborator Author

    !build

    @wujingyue wujingyue merged commit 9013e41 into main Jun 17, 2025
    17 checks passed
    @wujingyue wujingyue deleted the wjy/canlower branch June 17, 2025 05:25
    nsarka pushed a commit to nsarka/Fuser that referenced this pull request Jul 28, 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.

    2 participants