Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DT] Hoist constant source out of dispatch in HoistEncodingOps pass. #20125

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hanhanW
Copy link
Contributor

@hanhanW hanhanW commented Feb 27, 2025

The small constants can be inlined into dispatches, and they could become the source of set_encoding ops. The revision adds the support for hoisting both operations out if the source is a constant.

The small constants can be inlined into dispatches, and they could
become the source of `set_encoding` ops. The revision adds the support
for hoisting both operations out if the source is a constant.

Signed-off-by: hanhanW <[email protected]>
Copy link
Collaborator

@benvanik benvanik left a comment

Choose a reason for hiding this comment

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

heads up that you're going to want something more sophisticated - something along the lines of what we do for constexpr hoisting (you want to see if the entire slice of IR up to the root is constant-like and hoist the entire thing, not peepholing on the source of a set encoding op) - if this works (when not just looking at integers - thinking f16/fp8/etc) then fine for now

@@ -204,7 +204,14 @@ void HoistEncodingOpsPass::runOnOperation() {
});
IRRewriter rewriter(ctx);
for (auto setEncodingOp : candidates) {
Value src = setEncodingOp.getSource();
if (getConstantIntValue(src).has_value() &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

you want more than integers - you only care that the source is ConstantLike and can check for that (something like getDefiningOp()->hasTrait<ConstantLike>(), probably)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, thanks for pointing it out. I missed that the method only works for integers.

@benvanik benvanik self-requested a review February 28, 2025 00:02
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