lldb recently added a tablegen tool. In order to properly cross compile
lldb standalone there needs to be a mechanism to generate the native
lldb build, analgous to what's done for the NATIVE llvm build. Thus,
we can simply modify this setup to allow for any project to be used.
Details
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 35132 Build 35131: arc lint + arc unit
Event Timeline
cmake/modules/CrossCompile.cmake | ||
---|---|---|
86–92 | Does it hurt anything if these flags are included for other projects? |
I'm pretty sure that this doesn't work for Visual Studio (where you do not have a cross-compiler).
I'm pretty sure that this doesn't work for Visual Studio (where you do not have a cross-compiler).
It already doesn't work for Visual Studio. This change doesn't change that.
This just seems like it's replacing all instances of LLVM with ${CMAKE_PROJECT_NAME}?
I think it'd be cleaner and more explicit to pass an additional project name parameter to llvm_create_cross_target instead of implicitly using ${CMAKE_PROJECT_NAME}. There's some prior art for that, e.g. in https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/cmake/modules/AddLLVM.cmake;366290$1009?as=source&blame=off
cmake/modules/CrossCompile.cmake | ||
---|---|---|
58 | You'll wanna change this one to read from a project-specific cache variable too. You'll also wanna preserve backwards compatibility for the people using the old spelling. |
That's what I did originally. However, the users of build_natve_tool wouldn't have the proper context to know what to use. It's only non-llvm specific usage is within add_tablegen which would then have to learn how to decide what project_name variable to pass in. e.g. if lldb is part of ENABLE_PROJECTS then add_tablegen should call llvm_native_build(LLVM NATIVE ...) else llvm_native_build(LLDB NATIVE ...). So we'd have to propagate that to all users of add_tablegen where we'd have to figure out whether the build is standalone or not via checking CMAKE_PROJECT_NAME anyways. e.g.
if (${CMAKE_PROJECT_NAME} MATCHES lldb) add_tablegen(lldb lldb lldb-tblgen) else() add_tablegen(llvm lldb lldb-tblgen) endif()
I'm not convinced that adding the granularity to choose your llvm_create_cross_target project is worth propagating the configuration outwards given that there are no users that don't satisfy CMAKE_PROJECT_NAME == project_name.
An alternative would be to have some global variable llvm_cross_targets list that build_native_tool iterates over. But this is making an already clumsy solution more clumsy.
@smeenai I think if this change works then for now we can just land it because we have several bots to turn green in the short term. Would you be fine with allowing for this fix in the near term?
cmake/modules/CrossCompile.cmake | ||
---|---|---|
58 | I think you probably want to make CROSS_TOOLCHAIN_FLAGS_${target_name} generic, not project specific, and only add a project-specific variant if it is needed. It is entirely possible to use the cross-compile code to generate multiple different cross-projects for the same target where you do want the same target flags, because those flags relate to the target not the project you're building. |
I missed the build_native_tool thing. In that case, I would have been fine with sticking to ${CMAKE_PROJECT_NAME} inside llvm_create_cross_target as well, but it looks like you've already changed everything to use a project name variable :)
cmake/modules/CrossCompile.cmake | ||
---|---|---|
26 | Are you missing an underscore here? Should the variable be CROSS_TOOLCHAIN_FLAGS_${project_name}_${target_name}? | |
58 | Makes sense. I can see there being a need for project-specific variables as well. @lanza you should definitely keep the original ${CROSS_TOOLCHAIN_FLAGS_${target_name}} spelling here. You may also want to add the ${CROSS_TOOLCHAIN_FLAGS_${project_name}_${target_name}} spelling if e.g. there's some LLDB-specific configurations you'll want to pass? (You could just use the generic ${CROSS_TOOLCHAIN_FLAGS_${target_name}} for project-specific configs as well; it's slightly ugly, but as long as the configs don't cause issues in other projects it should be fine.) |
Are you missing an underscore here? Should the variable be CROSS_TOOLCHAIN_FLAGS_${project_name}_${target_name}?