-
Notifications
You must be signed in to change notification settings - Fork 3
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 system tweaks #246
base: main
Are you sure you want to change the base?
Build system tweaks #246
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
=======================================
Coverage 77.82% 77.82%
=======================================
Files 21 21
Lines 1028 1028
=======================================
Hits 800 800
Misses 228 228 ☔ View full report in Codecov by Sentry. |
As it turns out, there are some good reasons not to build tt-mlir in debug alongside tt-xla
As an alternative, I could add a separate cmake variable to control tt-mlir build type and default it to release, then users that want the extra info can easily set it trough the configure script. |
Ticket
Closes #232
What's changed
I made tt-xla pass down the build type to tt-mlir.
I extracted the directives specifying the compilers to use into a script that calls cmake. There was a typo in one of them, and they likely weren't doing anything as they were in the wrong place in the
CMakeLists.txt
(inspecting the final.so
showed mentions of both GCC 9 and Clang 17). This is one of the reasons why hardcoding a compiler in theCMakeLists.txt
is considered an anti-pattern. The new configure script is both a correctness and a QoL improvement.I changed the activate script to set
TTMLIR_TOOLCHAIN_DIR
to a default value(same one as tt-mlir default) instead of erroring when not set, as a QoL improvementI added a build convenience script
What hasn't changed
It's still hardcoded to use ccache for tt-mlir irrespective if it's used for tt-xla or not, I ran into build issues when trying to pass down
CMAKE_CXX_COMPILER_LAUNCHER
to tt-mlir in cases when it isn't set.Checklist