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

Minor update to the new MLIR specification for custom ops #8

Merged
merged 5 commits into from
Jan 2, 2023

Conversation

EiffL
Copy link
Contributor

@EiffL EiffL commented Nov 13, 2022

Following #7 , this small PR proposes to update this post to the new way that JAX uses to define custom ops by directly writing their specification using MLIR.

I'm pretty noob in MLIR but it seems very cool, and I guess at some point XLA will deprecate the old way of custom call building.
Here is the doc for the custom call op in MLIR: https://www.tensorflow.org/mlir/hlo_ops#mhlocustom_call_mlirmhlocustomcallop

And all the custom ops in jaxlib are now built this way.

Let me know if this looks good. I haven't run the checks on GPU

@dfm
Copy link
Owner

dfm commented Nov 13, 2022

@EiffL — Thank you!! I'm up to my eyeballs with work for the next little bit, but I'll take a look through and sanity check the GPU stuff ASAP.

@dfm
Copy link
Owner

dfm commented Nov 13, 2022

Also: @sharadmv and others might have thoughts on where this fits w.r.t. jax-ml/jax#12632 and its potential future implementation, but I still think it's probably sensible to update the doc here in the meantime!

* Update Dockerfile

* Update Dockerfile

* Update Dockerfile

* Update Dockerfile

* Update Dockerfile
@EiffL
Copy link
Contributor Author

EiffL commented Nov 13, 2022

No problem and no rush @dfm ^^ I've checked it works on GPU, and updated the docker cuda container to work with current jax version, should be all good :-)

@dfm
Copy link
Owner

dfm commented Dec 12, 2022

@EiffL — I'm finally taking a look at this. Sorry for being so slow. I made some tiny superficial changes, but I'm otherwise totally happy. One thought is that it might be useful to have more sensible words about the relationship between MLIR and XLA, and "translation" vs. "lowering", since both this document and the JAX docs are a little muddled about this, I'd say. But, I don't think that should stop us merging this. As soon as you sign off, @EiffL, I'm happy to merge. Thanks again!!

@dfm dfm merged commit 4796fee into dfm:main Jan 2, 2023
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.

2 participants