Skip to content
This repository has been archived by the owner on Jan 30, 2025. It is now read-only.

Move bottom up fuser declaration to header file #23

Merged
merged 6 commits into from
Dec 7, 2023

Conversation

mabubakarpurdue
Copy link
Contributor

Move bottom up fuser declaration to header file

No testing required since it's a minor restructuring

BUILD Outdated
@@ -18,6 +18,7 @@ td_library(
"include/mlir-tcp/Dialect/IR/TcpEnums.td",
"include/mlir-tcp/Dialect/IR/TcpOps.td",
"include/mlir-tcp/Dialect/IR/TcpTypes.td",
"include/mlir-tcp/Dialect/IR/TcpOpsCruiseInternal.td",
Copy link
Collaborator

Choose a reason for hiding this comment

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

unintentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. Fixed

BUILD Outdated
Comment on lines 198 to 199
"lib/Conversion/TorchToTcp/TorchToTcpCruiseInternal.cpp",
"lib/Conversion/TorchToTcp/CruiseInternalPatterns.cpp",
Copy link
Collaborator

Choose a reason for hiding this comment

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

please revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

std::function<void(Operation *, PatternRewriter &rewriter)>;

GenericBottomUpFuser(MLIRContext *context, CanFuseFuncType canFuseCallback,
PostProcessingFuncType postFuncCallback = nullptr)
Copy link
Collaborator

@sjain-stanford sjain-stanford Dec 7, 2023

Choose a reason for hiding this comment

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

Please add brief docstrings/comments on what the constructor args (canFuseCallback and postFuncCallback) are meant to do. Especially since the upstream passes (like TcpFuseElementwiseOpsPass) don't use the postFuncCallback, it is unclear from the source code in the header.

Copy link
Collaborator

@sjain-stanford sjain-stanford left a comment

Choose a reason for hiding this comment

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

Thanks and LGTM! Please address the docstring comment before landing.

@sjain-stanford sjain-stanford merged commit c4c94fb into cruise-automation:main Dec 7, 2023
1 check passed
srinathava pushed a commit to srinathava/mlir-tcp that referenced this pull request Jun 24, 2024
Move bottom up fuser declaration to header file

No testing required since it's a minor restructuring

---------

Co-authored-by: Muhammad Abubakar <muhammad.abubakar@getcruise.com>
srinathava pushed a commit to srinathava/mlir-tcp that referenced this pull request Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants