[Mlir-commits] [mlir] [mlir][cmake] Fix MLIR shared library installation (PR #152195)

[Christopher Bate]

================
@@ -388,6 +388,9 @@ function(add_mlir_library name)
 
   if(TARGET ${name})
     target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
+    if(ARG_INSTALL_WITH_TOOLCHAIN)
+      set_target_properties(${name} PROPERTIES MLIR_INSTALL_WITH_TOOLCHAIN TRUE)
----------------
christopherbate wrote:

>  However, I think it does require adding the INSTALL_WITH_TOOLCHAIN argument to the add_mlir_library_install function (since it contains the check  if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)).

One way would be to separate `add_mlir_library_install` into `add_mlir_library_install_impl` and `add_mlir_library_install`. That way you can hoist the checks up while `add_mlir_library_install` remains  the "public API" for users outside of this file. 

> Actually I am unable to get libMLIR.so* to be installed without the property, even when I add and use the add_mlir_library_install argument. In trying to do this, it seems a lot more intrusive and messy than adding the property. I am curious, what is the disadvantage of using a property

I don't know what the cost would be of the extra property. I did spend some time looking at LLVM/MLIR's CMake configuration traces  a while back and IIRC the amount of extra CMake code we're running for each target is a factor for the slowness. The build system goes through a ton of logic for each target right now. You could measure using before/after your change using a command like

```
time cmake ....original args...  --profiling-format=google-trace --profiling-output=cmake_trace.json
```

Anyway, I don't think this is a blocking issue. LGTM as long as it doesn't significantly lengthen configuration time (that we could otherwise avoid). 


https://github.com/llvm/llvm-project/pull/152195