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

Enable temporary shardy roudtrip import pass #279

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wooseokTT
Copy link

Ticket

Link to Github Issue

Problem description

Current JAX/openXLA is not natively support Shardy and stablehlo for compilation is not with fully Shardy dialect but with some openXLA intermediate format.

What's changed

This patch is a temporary solution that allows the openXLA intermediate form to be converted to Shardy dialect. Once openXLA supports Shardy natively in near future, this pass can be removed.

Checklist

  • New/Existing tests provide coverage for changes

@wooseokTT
Copy link
Author

This still needs further testing. I am just checking if it passes the pre-merge tests.

@wooseokTT wooseokTT added this to the [Multi Device 1] milestone Feb 25, 2025
Copy link

github-actions bot commented Feb 25, 2025

TestsPassed ❌️SkippedFailed
TT-XLA Tests0 ran0 passed0 skipped0 failed
TestResult
No test annotations available

@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2025

Codecov Report

Attention: Patch coverage is 24.75248% with 152 lines in your changes missing coverage. Please review.

Project coverage is 69.02%. Comparing base (a8769d7) to head (3501fa3).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/common/shardy_roundtrip_pass.cc 23.46% 150 Missing ⚠️
src/common/module_builder.cc 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
- Coverage   78.09%   69.02%   -9.07%     
==========================================
  Files          21       22       +1     
  Lines         986     1188     +202     
==========================================
+ Hits          770      820      +50     
- Misses        216      368     +152     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wooseokTT wooseokTT force-pushed the wooseok/shardy_roundtrip_pass branch from 07b387a to 3501fa3 Compare February 25, 2025 15:34
@wooseokTT
Copy link
Author

wooseokTT commented Feb 26, 2025

Google engineer reported to me that this pass can be included in Shardy repo by the end of this week (hopefully), so I am holding this PR to leverage the pass instead of creating shardy_roundtrip_pass.cc in tt-xla repo.

The code is ready but still need to wait for tt-mlir and shardy repo to incorporate the 'roundtrip' pipeline code.

@wooseokTT wooseokTT force-pushed the wooseok/shardy_roundtrip_pass branch from 3501fa3 to 2c23189 Compare February 28, 2025 14:43
@wooseokTT wooseokTT force-pushed the wooseok/shardy_roundtrip_pass branch from 2c23189 to 479f005 Compare March 3, 2025 16:56
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.

[Multi-device] Import Shardy 'roundtrip' mlir as a temporary solution
3 participants