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

Reenable LTC in out-of-tree build (for real this time) #1205

Merged
merged 3 commits into from
Aug 19, 2022
Merged

Reenable LTC in out-of-tree build (for real this time) #1205

merged 3 commits into from
Aug 19, 2022

Conversation

henrytwo
Copy link
Member

@henrytwo henrytwo commented Aug 9, 2022 •
edited
Loading

This PR addresses #1154 and #1126, which means that LTC can build OOT -- which allows it to be included in our wheels 🎉 🎉 .

To summarize, a problem we were hitting before was that OOT build of LTC in CI would fail due to TorchMLIRJITIRImporter being switched to a SHARED library, in order to access some functions from LTC. This resulted in some nasty nasty pybind errors.

I was unable to root cause the source of the failure from changing the library type to SHARED; however, this PR gets around this problem by breaking up TorchMLIRJITIRImporter into two parts: a core library, and the pybind. This way, the pybind can remain MODULE, which will prevent the original problem.

A new library called TorchMLIRJITIRImporterPybind has been created to handle the pybind, and TorchMLIRJITIRImporter has been converted to a static library -- due to problems linking on macOS-arm64 with a shared library. Detailed logs are available here,

Even with this, LTC on macOS-arm64 had been disabled to some other linkage issues, which will be addressed in the future here: #1253.

This PR also makes it so that LTC is built by default, now that it works for the majority of our CI configs. LTC has been explicitly disabled on macOS-arm64.

cc: @ke1337 @antoniojkim

@henrytwo henrytwo self-assigned this Aug 9, 2022
@henrytwo henrytwo mentioned this pull request Aug 9, 2022
Open
4 tasks
@henrytwo henrytwo changed the title Reenable LTC in out-of-tree build - round 2 Reenable LTC in out-of-tree build - here we go again Aug 9, 2022
@henrytwo henrytwo closed this Aug 16, 2022
@henrytwo henrytwo reopened this Aug 16, 2022
@henrytwo henrytwo force-pushed the henrytu/oot_ltc_fix branch 19 times, most recently from 46ab0d5 to 3a8e72a Compare August 19, 2022 00:17
@henrytwo henrytwo changed the title Reenable LTC in out-of-tree build - here we go again Reenable LTC in out-of-tree build (for real this time) Aug 19, 2022
@henrytwo henrytwo marked this pull request as ready for review August 19, 2022 01:06
powderluv
powderluv reviewed Aug 19, 2022
View reviewed changes
Copy link
Collaborator

@powderluv powderluv left a comment

Choose a reason for hiding this comment

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

Looks good. Just to verify can you try the nighty release build to make sure it passes since there is a setup.py change.

@henrytwo
Copy link
Member Author

henrytwo commented Aug 19, 2022 •
edited
Loading

Looks good. Just to verify can you try the nighty release build to make sure it passes since there is a setup.py change.

Yep, I'm running one now: https://github.com/llvm/torch-mlir/actions/runs/2888787539

@henrytwo henrytwo mentioned this pull request Aug 19, 2022
Open
@henrytwo
Copy link
Member Author

henrytwo commented Aug 19, 2022 •
edited
Loading

Looks good. Just to verify can you try the nighty release build to make sure it passes since there is a setup.py change.

Yep, I'm running one now: https://github.com/llvm/torch-mlir/actions/runs/2888787539

@silvasean @powderluv Release build ran thru successfully (only commits since then are comments, so it should be good to go)

Here's a rerun if we wanted to be safe: https://github.com/llvm/torch-mlir/actions/runs/2889907205

@henrytwo
Copy link
Member Author

@powderluv can I get some stamps on this please, if there's nothing that I need to change? Release build was successful on latest commit

@henrytwo henrytwo merged commit ba17a4d into main Aug 19, 2022
@henrytwo henrytwo deleted the henrytu/oot_ltc_fix branch August 19, 2022 19:25
@tanyokwok tanyokwok mentioned this pull request Sep 21, 2022
Closed
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
@tungld
Encode signatures and entry point names in EBCDIC codepage for z/OS (l…
22332bd 
…lvm#1205)

* Test

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

* Support EBCDIC encoding for zOSs

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

* Clean up

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

* Check mtriple

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

* Use llvm::Triple

Signed-off-by: Tung D. Le <tung@jp.ibm.com>

* sink charModeValue inside if stmt

Signed-off-by: Tung D. Le <tung@jp.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@powderluv powderluv powderluv approved these changes

@silvasean silvasean Awaiting requested review from silvasean

Assignees

@henrytwo henrytwo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@henrytwo @powderluv