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

module mlir supported on ROCm #99

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

weihanmines
Copy link
Contributor

Thanks for your contribution! Unfortunately, tensorflow/runtime is currently not
accepting contributions. Please see the
Contribution Guidelines for
more information.
@deven-amd @rsanthanam-amd

@weihanmines
Copy link
Contributor Author

@chsigg @hanbinyoon

@weihanmines
Copy link
Contributor Author

@chsigg The print error enum is added in this PR.

@weihanmines weihanmines changed the title Rocm module mlir module mlir supported on ROCm Mar 7, 2022
@chsigg
Copy link
Collaborator

chsigg commented Mar 8, 2022

Hi Wei, could you please explain why this change is necessary?

Also, please remove the unrelated changes.

Thanks!

@weihanmines
Copy link
Contributor Author

Hi Wei, could you please explain why this change is necessary?

Also, please remove the unrelated changes.

Thanks!

Hi Christian, the main purpose of this PR is to add jit compilation option for ROCm and get module.mlir working on ROCm. I will create two or three PRs later this week, so these PRs make reviewing the changes easier. Thank you.

@chsigg
Copy link
Collaborator

chsigg commented Mar 15, 2022

I think we came to the conclusion that HSACO is the appropriate serialization for a ROCm binary, even if it's architecture specific (PTX is forward compatible, but we could also use a FATBIN and update it for new archs).

The binary blob is only persistently stored in a few tests and does not need to be human-readable.

@weihanmines
Copy link
Contributor Author

I think we came to the conclusion that HSACO is the appropriate serialization for a ROCm binary, even if it's architecture specific (PTX is forward compatible, but we could also use a FATBIN and update it for new archs).

The binary blob is only persistently stored in a few tests and does not need to be human-readable.

Yes, we did. Thank you for the update.

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.

3 participants