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

Fix TOSA related breakages #2751

Open
sdasgup3 opened this issue Mar 19, 2025 · 2 comments
Open

Fix TOSA related breakages #2751

sdasgup3 opened this issue Mar 19, 2025 · 2 comments
Assignees

Comments

@sdasgup3
Copy link
Member

sdasgup3 commented Mar 19, 2025

Hi @Tai78641

The recent TOSA dialect changes introduced by https://github.com/llvm/llvm-project/pull/129720 have caused breakages, for example:

/usr/local/google/home/sdasgup/Github/stablehlo/stablehlo/conversions/tosa/transforms/TosaRescaleLegalizeToStablehlo.cpp:68:25: error: no member named 'getRoundingMode' in 'mlir::tosa::RescaleOp'                
  bool doubleRound = op.getRoundingMode() == "DOUBLE_ROUND";                                                                                                                                                       
                     ~~ ^                                                                                                                                                                                          
/usr/local/google/home/sdasgup/Github/stablehlo/stablehlo/conversions/tosa/transforms/TosaRescaleLegalizeToStablehlo.cpp:110:9: error: no viable conversion from '::llvm::ArrayRef<int32_t>' (aka 'ArrayRef<int>') 
to 'Value'                                                                                                                                                                                                         
  Value multiplier = op.getMultiplier();   

The PR is an attempt to address some of these issues, but it appears that the problem is more involving than we initially anticipated. Do you mind helping us updating the StablehloQuantLegalizeToTosaRescale.cpp and TosaRescaleLegalizeToStablehlo.cpp passes to accommodate these recent changes.

@sdasgup3
Copy link
Member Author

sdasgup3 commented Mar 19, 2025

#2752 disables certain build and test target to unblock integration while the while issue is fixed.

AI (@sdasgup3 ): Make sure to enable those targets once the current issue is resolved.

sdasgup3 added a commit that referenced this issue Mar 20, 2025
Other than the usual integration, this PR also addresses the followings:

1. Disables certain build and tests for
#2751.

In stablehlo/conversions/tosa/transforms/CMakeLists.txt, just commenting
the problematic files is not enough because

```
 LLVM's build system enforces that all source files are added to a build target
```

Hence we added add `PARTIAL_SOURCES_INTENDED` to the target
specification, though it is discouraged.

1. Fixes `build_tools/integrate/llvm_bump_revision.sh` to account for
unset value of `$1` as enforced by `set -o nounset`
@akuegel
Copy link
Member

akuegel commented Mar 21, 2025

I already have a fix for the compile errors, and adjusted the tests legalize_quant_ops_to_tosa_rescale.mlir and legalize_tosa_rescale_to_stablehlo.mlir
The other disabled tests are still failing and will need adjustments.

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

No branches or pull requests

3 participants