Skip to content
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

[libc] Improve get_object_files_for_test to reduce CMake configure time for tests. #75552

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

lntue
Copy link
Contributor

@lntue lntue commented Dec 15, 2023

Profiling cmake shows that a significant time configuring libc folder is spent on running get_object_files_for_test in the test folder (13 sec in libc/test folder / 16 sec in libc folder). By caching all needed objects for each target instead of resolving every time, the time cmake spends on configuring libc/test folder is reduced to ~1s.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 15, 2023

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Profiling cmake shows that a significant time configuring libc folder is spent on running get_object_files_for_test in the test folder (13 sec in libc/test folder / 16 sec in libc folder). By caching all needed objects for each target instead of resolving every time, the time cmake spends on configuring libc/test folder is reduced to ~1s.


Full diff: https://github.com/llvm/llvm-project/pull/75552.diff

1 Files Affected:

  • (modified) libc/cmake/modules/LLVMLibCTestRules.cmake (+48-30)
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 6cae0859149d54..44aacbcaf7951f 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -18,62 +18,80 @@ function(get_object_files_for_test result skipped_entrypoints_list)
   set(checked_list "")
   set(unchecked_list "${ARGN}")
   list(REMOVE_DUPLICATES unchecked_list)
-  list(LENGTH unchecked_list length)
 
-  while(length)
-    set(indirect_list "")
+  foreach(dep IN LISTS unchecked_list)
+    if (NOT TARGET ${dep})
+      # Skip tests with undefined dependencies.
+      list(APPEND skipped_list ${dep})
+      continue()
+    endif()
+    get_target_property(aliased_target ${dep} "ALIASED_TARGET")
+    if(aliased_target)
+      # If the target is just an alias, switch to the real target.
+      set(dep ${aliased_target})
+    endif()
 
-    foreach(dep IN LISTS unchecked_list)
-      if (NOT TARGET ${dep})
-        # Skip tests with undefined dependencies.
-        list(APPEND skipped_list ${dep})
-        continue()
-      endif()
-      get_target_property(dep_type ${dep} "TARGET_TYPE")
-      if(NOT dep_type)
-        # Skip tests with no object dependencies.
-        continue()
-      endif()
+    get_target_property(dep_type ${dep} "TARGET_TYPE")
+    if(NOT dep_type)
+      # Skip tests with no object dependencies.
+      continue()
+    endif()
+
+    get_target_property(dep_obj ${dep} "OBJECT_FILES_FOR_TESTS")
+    get_target_property(dep_skip ${dep} "SKIPPED_LIST_FOR_TESTS")
+    get_target_property(dep_checked ${dep} "CHECK_OBJ_FOR_TESTS")
+
+    if(dep_checked)
+      # Target full dependency has already been checked.  Just use the results.
+      list(APPEND object_files ${dep_obj})
+      list(APPEND skipped_list ${dep_skip})
+    else()
+      # Target full dependency hasn't been checked.  Recursively check its DEPS.
+      set(dep_obj "${dep}")
+      set(dep_skip "")
+
+      get_target_property(indirect_deps ${dep} "DEPS")
+      get_object_files_for_test(dep_obj dep_skip ${indirect_deps})
 
       if(${dep_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})
         get_target_property(dep_object_files ${dep} "OBJECT_FILES")
         if(dep_object_files)
-          list(APPEND object_files ${dep_object_files})
+          list(APPEND dep_obj ${dep_object_files})
         endif()
       elseif(${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE})
         get_target_property(is_skipped ${dep} "SKIPPED")
         if(is_skipped)
-          list(APPEND skipped_list ${dep})
-          continue()
+          list(APPEND dep_skip ${dep})
         endif()
         get_target_property(object_file_raw ${dep} "OBJECT_FILE_RAW")
         if(object_file_raw)
-          list(APPEND object_files ${object_file_raw})
+          list(APPEND dep_obj ${object_file_raw})
         endif()
       elseif(${dep_type} STREQUAL ${ENTRYPOINT_OBJ_VENDOR_TARGET_TYPE})
         # Skip tests for externally implemented entrypoints.
-        list(APPEND skipped_list ${dep})
-        continue()
+        list(APPEND dep_skip ${dep})
       endif()
 
-      get_target_property(indirect_deps ${dep} "DEPS")
-      list(APPEND indirect_list "${indirect_deps}")
-    endforeach(dep)
+      set_target_properties(${dep} PROPERTIES
+        OBJECT_FILES_FOR_TESTS "${dep_obj}"
+        SKIPPED_LIST_FOR_TESTS "${dep_skip}"
+        CHECK_OBJ_FOR_TESTS "YES"
+      )
 
-    # Only add new indirect dependencies to check.
-    list(APPEND checked_list "${unchecked_list}")
-    list(REMOVE_DUPLICATES indirect_list)
-    list(REMOVE_ITEM indirect_list checked_list)
-    set(unchecked_list "${indirect_list}")
-    list(LENGTH unchecked_list length)
-  endwhile()
+      list(APPEND object_files ${dep_obj})
+      list(APPEND skipped_list ${dep_skip})
+    endif()
+
+  endforeach(dep)
 
   list(REMOVE_DUPLICATES object_files)
   set(${result} ${object_files} PARENT_SCOPE)
   list(REMOVE_DUPLICATES skipped_list)
   set(${skipped_entrypoints_list} ${skipped_list} PARENT_SCOPE)
+
 endfunction(get_object_files_for_test)
 
+
 # Rule to add a libc unittest.
 # Usage
 #    add_libc_unittest(

@lntue lntue requested a review from jhuber6 December 15, 2023 00:56
@lntue lntue merged commit 791c5d0 into llvm:main Dec 18, 2023
4 checks passed
@lntue lntue deleted the cmake branch December 18, 2023 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants