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

[transforms] Fix simplification patterns for stablehlo.(and|or) #2638

Merged

Conversation

christopherbate
Copy link
Contributor

Fixes an issue in stablehlo-aggressive-simplification where %1 in
the below would get replaced by %arg0:

  %0 = stablehlo.constant dense<1> : tensor<2xi32>
  %1 = stablehlo.and %0, %arg0 : tensor<2xi32>

The pattern was checking whether %0 is equal to 0b1 and was
only tested on bools. A similar bug existed for stablehlo.and. Fixed
by just making sure the constant is integer with all bits set to 1.

Fixes an issue in `stablehlo-aggressive-simplification` where `%1` in
the below would get replaced by `%arg0`:

```
  %0 = stablehlo.constant dense<1> : tensor<2xi32>
  %1 = stablehlo.and %0, %arg0 : tensor<2xi32>
```

The pattern was checking whether `%0` is equal to `0b1` and was
only tested on bools. A similar bug existed for `stablehlo.and`. Fixed
by just making sure the constant is integer with all bits set to 1.
@christopherbate
Copy link
Contributor Author

Fixes #2633

@sdasgup3 sdasgup3 self-requested a review November 25, 2024 18:09
@christopherbate
Copy link
Contributor Author

@sdasgup3 @GleasonK Can someone help initiate CI workflows and/or merge the PR?

Copy link
Member

@GleasonK GleasonK left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Just getting back from OOO and catching up. Will take a look at your GH issue on the dynamic typing tomorrow shortly, should be able to tomorrow.

@GleasonK GleasonK merged commit 7a7baa0 into openxla:main Nov 25, 2024
10 checks passed
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.

3 participants