New issue
Advanced search Search tips

Issue 869136 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Unify logic for linking compiler-rt libraries, consider LLVM_ENABLE_PER_TARGET_RUNTIME_DIR

Project Member Reported by r...@chromium.org, Jul 30

Issue description

Chromium has a lot of gn logic to find compiler-rt libraries. Right now there are 7 hits in .gn[i] files:
https://cs.chromium.org/search/?q=clang_rt+file:.*%5C.gni?&sq=package:chromium&type=cs

It would be nice if we could calculate the library name and target architecture separately, so that we can write things like this:
  lib_paths = [ "$clang_base_path/lib/clang/lib/$clang_rt_triple" ]
  libs = [ "clang_rt.profile" ]

Instead of:
      if (is_win) {
        # Normally, we pass -fprofile-instr-generate to the compiler and it
        # automatically passes the right flags to the linker.
        # However, on Windows, we call the linker directly, without going
        # through the compiler driver. This means we need to pass the right
        # flags ourselves.
        _clang_rt_base_path =
            "$clang_base_path/lib/clang/$clang_version/lib/windows"
        if (target_cpu == "x86") {
          _clang_rt_suffix = "-i386.lib"
        } else if (target_cpu == "x64") {
          _clang_rt_suffix = "-x86_64.lib"
        }
        assert(_clang_rt_suffix != "", "target CPU $target_cpu not supported")
        ldflags += [ "$_clang_rt_base_path/clang_rt.profile$_clang_rt_suffix" ]
      } else {
        ldflags += [ "-fprofile-instr-generate" ]
      }

There will still be some complexity for copying around the DSOs needed for Mac and Windows, but it would be much simpler if we could separate the logic for calculating the triple from the filename.

I would start by refactoring the gn files so that there is a single helper for calculating clang runtime library paths, and then as a follow-up we can think about enabling LLVM_ENABLE_PER_TARGET_RUNTIME_DIR.
 

Sign in to add a comment