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

feat: add min to clamp and max to clamp canonicalizations. #86

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

ttjost
Copy link

@ttjost ttjost commented Jan 5, 2024

No description provided.

@ttjost ttjost requested a review from mgehre-amd January 5, 2024 15:16
@mgehre-amd
Copy link
Collaborator

From what I understand, you added two canonicalizers for clamp + min and clamp + max where in both cases the min/max have one scalar/splat operand.

Did you consider canonicalizing min/max with a scalar/splat operand to clamp (standalone),
and then a canonicalization that merges clamp + clamp into a single one?
This this more generic to me.

@ttjost
Copy link
Author

ttjost commented Jan 5, 2024

From what I understand, you added two canonicalizers for clamp + min and clamp + max where in both cases the min/max have one scalar/splat operand.

Did you consider canonicalizing min/max with a scalar/splat operand to clamp (standalone), and then a canonicalization that merges clamp + clamp into a single one? This this more generic to me.

Clamp + clamp already exists.
I thought about min+max canonicalization, but I thought that could come later. There will be a follow-up PR for it.

@ttjost ttjost force-pushed the tiagot.clamp_max_min_tosa_folding branch from ede7f80 to 89f16ea Compare January 5, 2024 17:38
@ttjost ttjost changed the title feat: add clamp->min and clamp->max canonicalizations. feat: add min to clamp and max to clamp canonicalizations. Jan 5, 2024
@ttjost
Copy link
Author

ttjost commented Jan 5, 2024

From what I understand, you added two canonicalizers for clamp + min and clamp + max where in both cases the min/max have one scalar/splat operand.

Did you consider canonicalizing min/max with a scalar/splat operand to clamp (standalone), and then a canonicalization that merges clamp + clamp into a single one? This this more generic to me.

Updated with Min to Clamp and Max to Clamp canoninicalizations.

@ttjost ttjost merged commit d0868e8 into feature/fused-ops Jan 5, 2024
1 check passed
@ttjost ttjost deleted the tiagot.clamp_max_min_tosa_folding branch January 5, 2024 17:54
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