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.

Sign up for GitHub

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

Jump to bottom

Port mhlo-quant-legalize-to-int pass to StableHLO #2383

Merged
merged 3 commits into from
Jun 10, 2024
Merged

Port mhlo-quant-legalize-to-int pass to StableHLO #2383

merged 3 commits into from
Jun 10, 2024

Conversation

sdasgup3
Copy link
Member

@sdasgup3 sdasgup3 commented Jun 7, 2024 •
edited
Loading

This is the port of mhlo-quant-legalize-to-int pass to Stablehlo.

A bunch of tests needs to be updates based on:

  • Different error message because of the differences in quantization verification for mhlo and stablehlo.
  • Update check directive to accommodate pretty printing in stablehlo vs not pretty printed ops in mhlo (like dot_general)
  • Modify tests with boradcast_in_dim to update the syntax of broadcast_dimension (from DenseIntElementsAttr to I64ArrayAttr)
  • Convert a test with per-axis stablehlo.max with per-tensor stablehlo.max as max only supports per-tensor. The intent of the tests is preserved.

Also, updated the original code to address creation of attributes of type I64ArrayAttr instead of DenseIntElementsAttr.

@sdasgup3 sdasgup3 marked this pull request as draft June 7, 2024 00:38
@sdasgup3 sdasgup3 marked this pull request as ready for review June 7, 2024 00:54
@sdasgup3 sdasgup3 requested a review from GleasonK June 7, 2024 01:09
GleasonK
GleasonK approved these changes Jun 7, 2024
View reviewed changes
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.

Awesome to see!! Thanks for porting this, very excited to have this in the repo for all to use / contribute to.

stablehlo/transforms/Passes.td Outdated Show resolved Hide resolved
stablehlo/transforms/Passes.td Outdated Show resolved Hide resolved
stablehlo/transforms/StablehloLegalizeQuantToInt.cpp Show resolved Hide resolved
stablehlo/tests/stablehlo_legalize_quant_to_int.mlir Show resolved Hide resolved
stablehlo/tests/stablehlo_legalize_quant_to_int.mlir Show resolved Hide resolved
This was referenced Jun 7, 2024
Closed
Merged
Closed
Merged
@sdasgup3 sdasgup3 merged commit 72feb5b into openxla:main Jun 10, 2024
10 checks passed
GleasonK pushed a commit that referenced this pull request Jun 10, 2024
@sdasgup3
Fix quant type handling in stablehlo-legalize-quant-to-int pass (#2385)
cca7073 
Originally brought up at
#2383 (comment)

The `stablehlo-legalize-quant-to-int` pass propagated from mho (#2383 )
uses std::variant to handle per-tensor and per-axis quantized type.
However, the base class quant::QuantizedType provides a better
polymorphic way to accomplish the same.

## Direction to reviewers

Please review the change only in the following commit:
ee8b499

Reason: This PR is based on
#2383 which is not merged yet
and hence has many files which are already reviewed as part of #2383.
sdasgup3 added a commit that referenced this pull request Jun 20, 2024
@sdasgup3
Interpreter support for quantized type (#2388)
6802016 
fixes #2373

The PR is rebased on top on
#2383 and cherry-pick changes
from #2384.

### Direction to reviewer

Please review the commit
4d7dc1a
**excluding** the following files
  - docs/generated/stablehlo_passes.md
  - stablehlo/transforms/Passes.td
  - stablehlo/transforms/ShapeLegalizeToStablehlo.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GleasonK GleasonK GleasonK approved these changes

Assignees
No one assigned
Labels
Quantization
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sdasgup3 @GleasonK