[libc-commits] [PATCH] D101991: [libc] Allow target architecture customization

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/cmake/modules/LLVMLibCLibraryRules.cmake:85
+  set_target_properties(${target_name}  PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+  target_compile_options(${target_name} PRIVATE ${LIBC_COMPILE_OPTIONS_DEFAULT})
 endfunction(add_entrypoint_library)
----------------
We should not need to set compile options here. If you notice, the `add_library` rule on line 79 does not compile anything. It just packs a bunch of object files into a static archive.


================
Comment at: libc/cmake/modules/LLVMLibCLibraryRules.cmake:115
+  set_target_properties(${target_name}  PROPERTIES LINKER_LANGUAGE "C")
+  target_compile_options(${target_name} PRIVATE ${LIBC_COMPILE_OPTIONS_DEFAULT})
 endfunction(add_redirector_library)
----------------
Same here as well. We should not need to set compile options.


================
Comment at: libc/cmake/modules/LLVMLibCObjectRules.cmake:155
 
-  set(common_compile_options -fpie ${LLVM_CXX_STD_default} -ffreestanding ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS})
+  set(common_compile_options -fpie ${LLVM_CXX_STD_default} -ffreestanding ${LIBC_COMPILE_OPTIONS_DEFAULT} ${ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS})
   set(internal_target_name ${fq_target_name}.__internal__)
----------------
While you are at it, can you fix this: we should use the same default compile options for `add_entrypoint_object` and `add_object_library` above. So, essentially, I am saying that we should be sharing `common_compiler_options` between these two rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101991