[PATCH] D90848: llvmbuildectomy - part 2

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

This looks like a major simplification of the build system. Code looks good, I confirmed it is working for my Windows build, Polly (i.e. statically linked pass plugins) is listed with `--libs`.

Since it is a relatively invasive change, you might want a second acceptance as well.



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:632-633
       llvm_map_components_to_libnames(llvm_libs
-       ${ARG_LINK_COMPONENTS}
-       ${LLVM_LINK_COMPONENTS}
-       )
+          ${ARG_LINK_COMPONENTS}
+          ${LLVM_LINK_COMPONENTS}
+      )
----------------
[nit] still unrelated whitespace change


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:639
+    # dependency information for this library through their name, and let
+    # LLVMBuildResolveComponentsLink reseolve the mapping.
     #
----------------
[typo] reseolve 


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:731
 
+# Define special targets that behave like component group. They don't have any
+# source attached but other component can add themselves to them. If the
----------------
[typo] article missing: **a** component group


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:732
+# Define special targets that behave like component group. They don't have any
+# source attached but other component can add themselves to them. If the
+# component supports is a Target and it supports JIT compilation, HAS_JIT must
----------------
[typo] component**s** 


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:735
+# be passed.
+function(add_llvm_component_group name)
+  cmake_parse_arguments(ARG "HAS_JIT" "" "LINK_COMPONENTS" ${ARGN})
----------------
Could you document here that `ADD_TO_COMPONENT` of `add_llvm_component_library ` is used to add a component to this group?


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:749
+#   - LLVM_COMPONENT_NAME: the name of the component, which can be the name of
+#     the associated library or the one speicified through COMPONENT_NAME
+#   - LLVM_LINK_COMPONENTS: a list of component this component depends on
----------------
[type] speicified 


================
Comment at: llvm/cmake/modules/LLVM-Build.cmake:41
+  foreach(llvmbuild_component ${llvmbuild_components})
+
+    if(llvmbuild_component STREQUAL "all")
----------------
[nit] stray whitespace


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90848/new/

https://reviews.llvm.org/D90848