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

Cleanup: Remove duplicate quantization checks #2566

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

sdasgup3
Copy link
Member

@sdasgup3 sdasgup3 commented Sep 30, 2024

The recent upstream change have introduced quantization checks that are
already present in the StableHLO core library. This commit removes these duplicate
checks to avoid redundancy and potential inconsistencies.

Checks proposed to be removed StableHLO Code Upstream MLIR
channel-axis >= 0 cs cs
scale within smallest and largest finite numbers determined by expressed_type cs cs1 cs2

Note that StableHLO has checks like quantization_dimension < rank(self) and
dim(self, quantization_dimension) = size(scales) implemented at cs. In upstream MLIR similar checks cs are encoded as part of dcast and qcast ops and hence cannot be claimed as duplicate.

related upstream clean-up llvm/llvm-project#110604

@sdasgup3
Copy link
Member Author

sdasgup3 commented Sep 30, 2024

@abhigunj We can merge this "after" the LLVM integrate bringing the change.

@sdasgup3 sdasgup3 marked this pull request as draft September 30, 2024 23:23
@abhigunj
Copy link
Member

@abhigunj We can merge this once "after" the LLVM integrate bringing the change.

Thank you for taking care of the clean up, I was planning to do it during LLVM revision bump.

@sdasgup3
Copy link
Member Author

Thanks @abhigunj!

As part of integrate we still have to update the quantized tests to fix the error messages. This change can land later as these will be unused at that point.

@sdasgup3 sdasgup3 marked this pull request as ready for review October 3, 2024 23:10
@abhigunj abhigunj merged commit c495957 into openxla:main Oct 3, 2024
10 checks passed
@@ -751,23 +751,6 @@ bool isValidStablehloQuantizedElementType(Type elementType) {
quantizedPerAxisElementType.getScales().end());
}

// quantized_type_c6
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this to a TD file? Or somewhere else for searchability?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants