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

[BUILD] Support building out of tree plugins #3007

Merged
merged 20 commits into from
Feb 12, 2024

Conversation

nhat-nguyen
Copy link
Collaborator

@nhat-nguyen nhat-nguyen commented Jan 24, 2024

This PR adds support for building out of tree plugins with triton.
External plugins can provide an environment variable before invoking python setup.py to let triton know where to find the backends.

  • TRITON_PLUGIN_DIRS: semicolon-separated list of directories containing the plugins. The last components of the paths will be used as plugin names.

For instance, if we have triton_shared under /home/user/github/triton_shared, to build triton_shared with triton, we can do:

export TRITON_PLUGIN_DIRS="/home/user/github/triton_shared"
python3 setup.py build

This also assumes that the external backends have moved over to the new external backend architecture; they have to expose a backend folder under their root directory.

With this change, all external backends will still be copied to triton/runtime and be used like:

import triton
from triton.backends.triton_shared.driver import CPUDriver

triton.runtime.driver.set_active(CPUDriver())

CMakeLists.txt Outdated Show resolved Hide resolved
python/setup.py Outdated Show resolved Hide resolved
python/setup.py Outdated Show resolved Hide resolved
python/setup.py Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@manbearian
Copy link
Collaborator

@nhat-nguyen Thanks for doing this work!

The changes look good; i can confirm this should work for the OSS triton-shared project as well as what Microsoft needs to support Maia 100.

@ptillet
Copy link
Collaborator

ptillet commented Jan 24, 2024

seems like there is a lot of copy pasted code between copy_backends and copy_external_backends

@etiotto
Copy link
Contributor

etiotto commented Jan 24, 2024

This PR provides a way for external backends to be built and included into Triton so it has value. Users will need to clone the external backend and so they will actually need to be aware that the backend exists somewhere in a different github repository.

I personally would prefer an external backend to be a submodule. This has the advantage of not requiring users to explicitly clone the third party code, and makes it immediately clear that the backend exists. If the submodule is a complete fork of the OpenAI code (plus vendor changes of course) there would be no maintenance costs in the OpenAI project and no extra efforts for the end users of the project.

@ptillet
Copy link
Collaborator

ptillet commented Jan 25, 2024

I think this looks overengineered now... I am not convinced that setup.py needs +60lines to discovered external backends...

@nhat-nguyen
Copy link
Collaborator Author

I think this looks overengineered now... I am not convinced that setup.py needs +60lines to discovered external backends...

Thanks! I just simplified this the best I could and kept almost the same code structure as before. Let me know what you think!

@nhat-nguyen
Copy link
Collaborator Author

@ptillet When you have some cycles, would you mind taking a look at this again? Thank you!

@nhat-nguyen nhat-nguyen changed the title Support building out of tree plugins [BUILD] Support building out of tree plugins Jan 31, 2024
nhat-nguyen added a commit to microsoft/triton-shared that referenced this pull request Feb 1, 2024
This PR updates the reference CPU backend using the triton-shared middle-layer to use the latest triton 3rd party integration.

## Notable changes
+ `backend/driver.py` and `backend/compiler.py` implement the compiler and driver interfaces required to work with triton.
+ `triton-shared` now includes `triton` as its submodule. The submodule currently points to triton-lang/triton#3007 and will be updated to use the `main` branch once the linked PR is merged.
+ to let triton know we're using the CPU backend, we need to do `triton.runtime.set_active(CPUBackend())` -- see the updated `python/examples` folder for details.

## Other fixes
+ Fixes in `BufferizableOpInterface` due to llvm update in triton main.
+ Lit tests updated due to the above change.
+ previously we could build triton using `python setup.py build`, but that is no longer working after the recent update. We have to do `python3 -m pip install --no-build-isolation -vvv '.[tests]'` directly.
@nhat-nguyen
Copy link
Collaborator Author

@ptillet I played around with this idea a bit more and would like to hear what you think.

I feel automatically using the last component of the plugin path isn't good a good dev experience since it requires the directory to also be a valid python identifier (for when we actually copy the backend dirs to triton/runtime during the build). So backends containing dashes - have to be renamed during clone to use correctly.

Using a list like name1,path1;name2,path2 is also error-prone and makes the cmake / python code more complicated because we have to parse strings; paths can't also contain commas otherwise everything will be messed up.

What are your thoughts on requiring the plugin backend to contain a file describing the name to use? Like in the case of triton-shared, we will put something under triton-shared/backend a file called name.conf with the python module name to be used? Thank you!

@aaronsm
Copy link
Contributor

aaronsm commented Feb 3, 2024

I think this could be done similar to LLVM_TARGETS_TO_BUILD.

$ TRITON_CODEGEN_BACKENDS="amd;nvidia;triton-shared" \
  pip3 install -e python

Here are example changes -- https://github.com/aaronsm/triton/commit/17c6f6f08d8f835d30c9cf0294f02f57c0b679e1

@nhat-nguyen
Copy link
Collaborator Author

nhat-nguyen commented Feb 4, 2024

I think this could be done similar to LLVM_TARGETS_TO_BUILD.

$ TRITON_CODEGEN_BACKENDS="amd;nvidia;triton-shared" \
  pip3 install -e python

Here are example changes -- aaronsm@dbecc92

The issue though is both triton-shared and intel xpu are no longer in-tree. Only nvidia and amd are.

During the last community meeting we had a discussion about this but didn’t reach a definitive conclusion.

Phil mentioned that third-party plugins will now have to include triton as a submodule. With this requirement, we will need a way to tell triton where to build the plugins through an env variable.

We can still definitely reuse the old approach like you suggested, but that means 1) we will now have a fork of triton that includes the source code of the backend and 2) it doesn’t quite match the structure that Phil suggested: having triton as a submodule.

@nhat-nguyen nhat-nguyen merged commit 7acb4ff into main Feb 12, 2024
4 checks passed
@nhat-nguyen nhat-nguyen deleted the nhat/new_plugin_triton_submodule branch February 12, 2024 17:53
binarman pushed a commit to binarman/triton that referenced this pull request Apr 2, 2024
This PR adds support for building out of tree plugins with triton.
External plugins can provide an environment variable before invoking
`python setup.py` to let triton know where to find the backends.

+ `TRITON_PLUGIN_DIRS`: semicolon-separated list of directories
containing the plugins. The last components of the paths will be used as
plugin names.

For instance, if we have `triton_shared` under
`/home/user/github/triton_shared`, to build `triton_shared` with
`triton`, we can do:

```
export TRITON_PLUGIN_DIRS="/home/user/github/triton_shared"
python3 setup.py build
```

This also assumes that the external backends have moved over to the new
external backend architecture; they have to expose a `backend` folder
under their root directory.

With this change, all external backends will still be copied to
`triton/runtime` and be used like:

```python
import triton
from triton.backends.triton_shared.driver import CPUDriver

triton.runtime.driver.set_active(CPUDriver())
```
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.

None yet

5 participants