The flang(f18) developers desire a way to build llvm and mlir separately for installation and then build the flang front-end out of tree. This patch adds some cmake infrastructure to allow that development environment to behave correctly.
Details
Diff Detail
Event Timeline
Nice, seems to match what clang does here too so that's good to keep it in sync
mlir/CMakeLists.txt | ||
---|---|---|
8–9 | Cant this be removed now? | |
39–40 | Can this be removed now? | |
mlir/cmake/modules/CMakeLists.txt | ||
2–4 | I was going to comment "What does clang do?" and then I saw they have this exact same comment ;-) | |
29–30 | Nit: could you indent these to show the nesting? | |
36 | Same re indent | |
44–47 | Leftover from testing? | |
52 | Could you add a comment for this section? |
mlir/CMakeLists.txt | ||
---|---|---|
8–9 | Possibly. We can give it a try. | |
mlir/cmake/modules/CMakeLists.txt | ||
29–30 | Sure. :) | |
44–47 | This was leftover from cobbling things together from other LLVM projects. Other projects seem to setup this XXXConfig.cmake file, but I found it wasn't needed here. We can remove these commented out lines, if you prefer. I left them with the chance someone who knew CMake better had good reasons to build out an MLIRConfig.cmake rule. | |
52 | I can give it a shot. |
I think you may have forgotten to upload the changed diff :) In general I think this looks good, I'd like to remove the duplicate functions but beyond that this should be good.
mlir/cmake/modules/CMakeLists.txt | ||
---|---|---|
44–47 | Lets remove them, instead thanks. |
mlir/cmake/modules/CMakeLists.txt | ||
---|---|---|
29–30 | Actually, this looks like it is in column-0 because it is quoted. I'm going to leave it. |
Do you have the ability to use arc by the way? It enables the CI testing to build/test revision here
This patch breaks Windows MSVC build, former correct ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${LIB}.lib now suddenly becomes wrong ${LIB}.
I'll revert this part to what it was given https://bugs.llvm.org/show_bug.cgi?id=44660
Cant this be removed now?