[Docs] (WIP) Add a new page to help for first time contributors#426
[Docs] (WIP) Add a new page to help for first time contributors#426v01dXYZ wants to merge 11 commits intoGenesis-Embodied-AI:mainfrom
Conversation
|
I don't agree with building LLVM oneself. It is very heavy and difficult. I never have to do this. Please can you look at how our CI runs the build, and identify what is different between your current build process and the CI build process? |
|
(meaning how Quadrants runs the build I mean) |
|
Sorry to disagree on this but I don't think it is really hard to build LLVM as it is mainly Why is it fine to document it? Sometimes, there could be some bugs in LLVM and it is great to have a version that could output the intermediate passes. or a debug version (that one is the really hard one as LLVM debug symbols are HUGE). The CI build process uses |
Yes, it takes a long time to build.
It's documented by the CI at https://github.com/Genesis-Embodied-AI/quadrants-sdk-builds/blob/main/.github/workflows/llvm-ci.yml I feel. I'm fine with linking to this in some "advanced, here be dragons" section. But not as a 'this is the standard build process' doc.
Yes. It should just download and work. I'd like a diagnosis into why this doesn't work for you first please, before considering any documentation beyond an 'advanced, here be dragons' type section, please. |
|
:D I didn't say it doesn't work. I went straight to that as I wanted to understand why Instruction Selection failed. But yes, it is for advanced users that want to find out why LLVM panics or if some passes could be better implemented. Thinking again you're right, let's cut it off. |
|
Oh, I see, you are covering multiple approaches:
I actually missed that you were covering two other approaches, on initial skimming, to be honest. Note that the build process as far as llvm and clang is a little bit confused currently, and could probably be cleaned up a little. We actually use three somewhat orthogonal things:
The C/C++ compiler is pretty much always using a platform-native compiler, e.g. gcc, or msvc, currently. Which seems reasonable I feel. This is defined here The clang/clang++ compiler currently uses an OS-installed clang/clang++ on all platforms, I believe though I was pushing to use the downloaded clang/clang++ for a while. I saw advantages/disadvantages as: good points of using packaged/downloaded clang/clang++:
downsides of using packaged/downloaded clang/clang++:
Lastly we have the LLVM libraries themselves, which are always downloaded, by default. They are linked with, when building Quadrants. This is the only bit which must be a specific version, and that is tightly coupled with the code, AFAIK. Anyway... I'm not averse to having a section on building clang and/or llvm yourself, if you feel this is useful, but should clearly be marked as 'advanced here be dragons' (or similar) I feel. It would probably be worth documenting clearly the difference between the three things above, somehow, somewhere? |
|
My only intent is to debug the LLVM code that is called by the library. But what you said is worth mentioning about what is compiled. To reformulate,
|
|
|
||
| Concerning the extra cmake arguments, the notable ones are: | ||
|
|
||
| * `CMAKE_CXX_COMPILER`: it should be `clang++` |
There was a problem hiding this comment.
Are you sure this is needed? I never set this myself.
I simply do:
./build.py --shell
python setup.py develop
There was a problem hiding this comment.
What does --shell? Does it activate a venv with all the deps?
There was a problem hiding this comment.
it sets a bunch of env vars
there's also a version for AI. I don't pay close attention to what AI does, but it seems to be doing somethign like:
./build.py -w env.sh
source env.sh
python setup.py develop
|
|
||
| ## Notes | ||
|
|
||
| * *testing*: Any new features or modified code should be tested. You have to run the test suite using `./tests/run_tests.py` which set up |
| ## Notes | ||
|
|
||
| * *testing*: Any new features or modified code should be tested. You have to run the test suite using `./tests/run_tests.py` which set up | ||
| the right test environment for `pytest`. CLI arguments are forwarded to `pytest`. Do not use `pytest` directly as it behaves differently. |
| of a compiler project. We can easily mix things up. So let' explicitly write down the CI convention about the compilers/LLVM that are used for the three pieces: | ||
|
|
||
| 1. `quadrants` host runtime: compiled using the OS default C/C++ compiler. | ||
| 2. `quadrants` device runtime (bitcode): compiled using `clang` from the distribution/OS. Using `clang` is required as it has to support the same targets as `LLVM` (obviously!). |
There was a problem hiding this comment.
nit: I think it uses clang++?
|
|
||
| ### CI Convention about compilers/LLVM | ||
|
|
||
| `quadrants` is by itself somewhat a compiler targeting CPUs and GPUs altogether. There are at least three pieces that need to be compiled or are themselves part |
There was a problem hiding this comment.
Lets keep just the second sentence, to keep as concise as posible "There are three pieces that need to be compiled or are themselves part
of a compiler project.". Actually we can shorten further: "Quadrants comprises three parts:".
But actually, what about the python code that is part of Quadrants? One can argue it is included in the 'host runtime' part. But on the other hand, below it says that the 'host runtime' is compiled, usng a C/C++ compiler. Thoughts?
There was a problem hiding this comment.
(I am wary of saying that Quadrants 'comprises 3 parts' to be honest. There are also .pyi stub files from pybind11, for example. Thoughts?)
|
Just to check, you are still addressing the comments right? Not expecting anything from me on this currently I think? |
|
Thanks for asking. Yes, I addressed your review. You can take a look if you want to point to place where the style could be improved (maybe I should make it more formal). I also opened an issue about ditch |
|
|
||
| ## Getting the right python version with an isolated dependency tree | ||
|
|
||
| It is recommended to use a virtual env and the right python version (currently `3.10`). `pre-commit` for instance is configured with a pinned Python version. |
There was a problem hiding this comment.
I develop using 3.10, but we officially support 3.10.3.13 currently, see
| `build.py` can be used at least two ways: | ||
|
|
||
| * `build.py wheel` to build the wheel currently using `setup.py bdist_wheel` | ||
| * `build.py --shell` to enter a shell with environment variables set up as with `build.py wheel` in order to let you invoke yourself the commands. |
There was a problem hiding this comment.
there is also the AI way, where it outputs a script to set the env vars.
There was a problem hiding this comment.
this seems not to have been addressed yet?
There was a problem hiding this comment.
No, I've got to take a look at it before answering something relevant.
| * `build.py wheel` to build the wheel currently using `setup.py bdist_wheel` | ||
| * `build.py --shell` to enter a shell with environment variables set up as with `build.py wheel` in order to let you invoke yourself the commands. | ||
|
|
||
| For instance, if you want to tinker the `quadrants` python runtime without rebuilding every time, you |
There was a problem hiding this comment.
"python setup.py develop provides incremental builds. Use like:"
|
|
||
| ## Building the package for release purposes | ||
|
|
||
| As previously mentionned, to build the release package, you can invoke: |
There was a problem hiding this comment.
Actually, just "To build the release package:"
| ``` | ||
|
|
||
| If you are interested about the environment variables set up by `build.py` | ||
| (or you need them for one of your script), you can write them down to a file |
There was a problem hiding this comment.
"write them to a file"
| python setup.py develop | ||
| ``` | ||
|
|
||
| If you are interested about the environment variables set up by `build.py` |
There was a problem hiding this comment.
"To see the environment variable set by build.py"
There was a problem hiding this comment.
Actually, let's just shorten this paragraph to:
"To write the environment variables to a file use python setup.py -w [filename]. For example:"
| ./build.py wheel | ||
| ``` | ||
|
|
||
| We use `cmake` to build the C++ core. The build directory depends on the host architecture and the python version: it could be for instance `_skbuild/linux-x86_64-3.10/cmake-build`. |
There was a problem hiding this comment.
"version: it could be for instance" => "version. For example:"
|
|
||
| We use `cmake` to build the C++ core. The build directory depends on the host architecture and the python version: it could be for instance `_skbuild/linux-x86_64-3.10/cmake-build`. | ||
|
|
||
| You can modify the cmake options to your liking in order to enable or disable some features you need or don't need. To discover them, use `ccache`: |
There was a problem hiding this comment.
"use ccache" => "you can use ccache or ccmake"
There was a problem hiding this comment.
Well, I made a mistake. It's ccmake.
|
|
||
| ``` | ||
| ccache _skbuild/linux-x86_64-3.10/cmake-build | ||
| ``` |
There was a problem hiding this comment.
lets include also the ccmake command ie
(cd _skbuild/linux-x86_64-3.10/cmake-build; ccmake $PWD)
| ccache _skbuild/linux-x86_64-3.10/cmake-build | ||
| ``` | ||
|
|
||
| You could then set the environment variable `QUADRANTS_CMAKE_ARGS` that will be appended to the `cmake` command used to configure the `cmake` build. |
There was a problem hiding this comment.
do you mean, when we run ./build.py --shell?
There was a problem hiding this comment.
it is appended by setup.py before invoking cmake.
To me, it doesn't seem related to build.py --shell.
There was a problem hiding this comment.
I've never run cmake directly to be honest. I've always done ./build.py --shell. I have no objection to making the build more and more standard, but I would like some explanation please of:
- why are you not simply using ./build.py --shell and python setup.py develop?
- how does calling cmake direclty relate to the above?
There was a problem hiding this comment.
- I didn't know
build.pyexisted and I used at firstpipas it is the way. But now I usebuild.py. - I don't call
cmakedirectly as I have then to copy manually the shared object. But using thecmakeoptions is required to disable some unwanted backends. It's not necessary though.
|
|
||
| Quadrants comprises at least three important parts: | ||
|
|
||
| 1. `quadrants` host runtime: Made with a mix of Python and C++. The C++ core is compiled using the OS default C/C++ compiler. |
| Quadrants comprises at least three important parts: | ||
|
|
||
| 1. `quadrants` host runtime: Made with a mix of Python and C++. The C++ core is compiled using the OS default C/C++ compiler. | ||
| 2. `quadrants` device runtime (bitcode): C++ code compiled using `clang++` from the distribution/OS. Using `clang` is required as it has to support the same targets as `LLVM` (obviously!). |
There was a problem hiding this comment.
lets remove (obviously!) (since if it was obvious we wouldn't need to say it, I feel).
Arent we using clang++ rather than clang? (correct me if I'm wrong on this point.
There was a problem hiding this comment.
In my mind, clang refers to the project while clang++ refers to the CLI command. That's why I left this clang. But let's be consistent.
|
|
||
| 1. `quadrants` host runtime: Made with a mix of Python and C++. The C++ core is compiled using the OS default C/C++ compiler. | ||
| 2. `quadrants` device runtime (bitcode): C++ code compiled using `clang++` from the distribution/OS. Using `clang` is required as it has to support the same targets as `LLVM` (obviously!). | ||
| 3. `LLVM` libraries used by host runtime: statically or dynamically linked, used to lower the kernel's final IR to machine code on the host. The CI uses an LLVM version compiled from source. |
There was a problem hiding this comment.
They're used by a lot more than final lowering. They're used throughout the entire c++ side, that receives the python side AST, via pybind11 api calls, transforms them to CHIIR, optimizes them, then gradually lowers them to LLVM IR.
There was a problem hiding this comment.
I'll take a look at that to be more accurate.
There was a problem hiding this comment.
This hasnt been addressed yet right?
|
|
||
| ### Building LLVM for debugging it | ||
|
|
||
| Sometimes, it could be useful to have a LLVM version that allows to print intermediate passes or with debug symbols to find out where and why LLVM fails (for example, when Instruction Selection fails). |
There was a problem hiding this comment.
To clarify to my own mind, what is possible here that is not possible using QD_DUMP_IR=1?
There was a problem hiding this comment.
No because we will only debug quadrants not LLVM. If we want to follow after each pass what does LLVM (ie when we dohas module->lookup("symbol")), it has to be with a LLVM build that kept LLVM_DEBUG statements.
| If you are interested about the environment variables set up by `build.py` | ||
| (or you need them for one of your scripts), you can write them down to a file | ||
| to source it by yourself: | ||
| To write the environment variables to a file, use `./build.py -w [filename]`. For example: |
| ## Building the package for release purposes | ||
|
|
||
| As previously mentionned, to build the release package, you can invoke: | ||
| To build the release package: |
| ``` | ||
|
|
||
| We use `cmake` to build the C++ core. The build directory depends on the host architecture and the python version: it could be for instance `_skbuild/linux-x86_64-3.10/cmake-build`. | ||
| We use `cmake` to build the C++ core. The build directory depends on the host architecture and the python version. For example: `_skbuild/linux-x86_64-3.10/cmake-build`. |
|
|
||
| ``` | ||
| ccache _skbuild/linux-x86_64-3.10/cmake-build | ||
| ccmake _skbuild/linux-x86_64-3.10/cmake-build |
There was a problem hiding this comment.
Does this work? I think the command is:
(cd _skbuild/linux-x86_64-3.10/cmake-build; ccmake $PWD)
Note: $PWD here does NOT resolve to _skbuild/linux-x86_64-3.10/cmake-build, it resolves to the pwd before running the command.
There was a problem hiding this comment.
On my side, it does work.
There was a problem hiding this comment.
Try both approaches. You'll see the options available are different.
There was a problem hiding this comment.
Interesting. Seems they give the same result.
There was a problem hiding this comment.
Sorry for being a little pr*ck but the reason is https://cmake.org/cmake/help/latest/manual/ccmake.1.html
There was a problem hiding this comment.
That's a very long document. Please could you isolate the relevant 2-3 lines or so.
There was a problem hiding this comment.
The synopsis shows the command could be used with argv[1] := <path_to_cmake_build>.
There was a problem hiding this comment.
Oh I see. And you are confident this won't break/change anything?
There was a problem hiding this comment.
no, I don't think it breaks things compared to doing a cd + $PWD.
|
Just to check, have you addressed all comments, and would like a fresh review? Alternatively, once you believe you have addressed all comments, and would like a fresh review, please add a comment stating that. |
Issue: No issue
Brief Summary
My first contribution was a little bit harder than expected. So I wrote down some pieces of advice for new contributors.
It is a WIP and you're welcome to edit it!