[PATCH] D13326: [PGO]: Eliminate __llvm_profile_register calls for Linux (clang changes)
Justin Bogner via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 6 00:13:09 PDT 2015
David Li <davidxl at google.com> writes:
> davidxl updated this revision to Diff 36316.
> davidxl added a comment.
>
> I have modified the implementation to not use linker script, so this
> clang patch becomes strictly refactoring with NFC. I think it is still
> a good thing to have this in so that similar tunings like this can be
> done in the future.
Sure. I have a couple of nitpicks but this basically LGTM.
>
> http://reviews.llvm.org/D13326
>
> Files:
> include/clang/Driver/ToolChain.h
> lib/Driver/SanitizerArgs.cpp
> lib/Driver/ToolChain.cpp
> lib/Driver/ToolChains.cpp
> lib/Driver/ToolChains.h
> lib/Driver/Tools.cpp
>
> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -2402,83 +2402,12 @@
> }
> }
>
> -// Until ARM libraries are build separately, we have them all in one library
> -static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
> - const ArgList &Args) {
> - const llvm::Triple &Triple = TC.getTriple();
> - bool IsWindows = Triple.isOSWindows();
> -
> - if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
> - return "i386";
> -
> - if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
> - return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
> - ? "armhf"
> - : "arm";
> -
> - return TC.getArchName();
> -}
> -
> -static SmallString<128> getCompilerRTLibDir(const ToolChain &TC) {
> - // The runtimes are located in the OS-specific resource directory.
> - SmallString<128> Res(TC.getDriver().ResourceDir);
> - const llvm::Triple &Triple = TC.getTriple();
> - // TC.getOS() yield "freebsd10.0" whereas "freebsd" is expected.
> - StringRef OSLibName =
> - (Triple.getOS() == llvm::Triple::FreeBSD) ? "freebsd" : TC.getOS();
> - llvm::sys::path::append(Res, "lib", OSLibName);
> - return Res;
> -}
> -
> -SmallString<128> tools::getCompilerRT(const ToolChain &TC, const ArgList &Args,
> - StringRef Component, bool Shared) {
> - const char *Env = TC.getTriple().getEnvironment() == llvm::Triple::Android
> - ? "-android"
> - : "";
> -
> - bool IsOSWindows = TC.getTriple().isOSWindows();
> - bool IsITANMSVCWindows = TC.getTriple().isWindowsMSVCEnvironment() ||
> - TC.getTriple().isWindowsItaniumEnvironment();
> - StringRef Arch = getArchNameForCompilerRTLib(TC, Args);
> - const char *Prefix = IsITANMSVCWindows ? "" : "lib";
> - const char *Suffix =
> - Shared ? (IsOSWindows ? ".dll" : ".so") : (IsITANMSVCWindows ? ".lib" : ".a");
> -
> - SmallString<128> Path = getCompilerRTLibDir(TC);
> - llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" +
> - Arch + Env + Suffix);
> -
> - return Path;
> -}
> -
> -static const char *getCompilerRTArgString(const ToolChain &TC,
> - const llvm::opt::ArgList &Args,
> - StringRef Component,
> - bool Shared = false) {
> - return Args.MakeArgString(getCompilerRT(TC, Args, Component, Shared));
> -}
> -
> // This adds the static libclang_rt.builtins-arch.a directly to the command line
> // FIXME: Make sure we can also emit shared objects if they're requested
> // and available, check for possible errors, etc.
> static void addClangRT(const ToolChain &TC, const ArgList &Args,
> ArgStringList &CmdArgs) {
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, "builtins"));
> -}
> -
> -static void addProfileRT(const ToolChain &TC, const ArgList &Args,
> - ArgStringList &CmdArgs) {
> - if (!(Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
> - false) ||
> - Args.hasArg(options::OPT_fprofile_generate) ||
> - Args.hasArg(options::OPT_fprofile_generate_EQ) ||
> - Args.hasArg(options::OPT_fprofile_instr_generate) ||
> - Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> - Args.hasArg(options::OPT_fcreate_profile) ||
> - Args.hasArg(options::OPT_coverage)))
> - return;
> -
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, "profile"));
> + CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
> }
>
> namespace {
> @@ -2559,7 +2488,7 @@
> // whole-archive.
> if (!IsShared)
> CmdArgs.push_back("-whole-archive");
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, Sanitizer, IsShared));
> + CmdArgs.push_back(TC.getCompilerRTArgString(Args, Sanitizer, IsShared));
> if (!IsShared)
> CmdArgs.push_back("-no-whole-archive");
> }
> @@ -2569,7 +2498,7 @@
> static bool addSanitizerDynamicList(const ToolChain &TC, const ArgList &Args,
> ArgStringList &CmdArgs,
> StringRef Sanitizer) {
> - SmallString<128> SanRT = getCompilerRT(TC, Args, Sanitizer);
> + SmallString<128> SanRT = TC.getCompilerRT(Args, Sanitizer);
> if (llvm::sys::fs::exists(SanRT + ".syms")) {
> CmdArgs.push_back(Args.MakeArgString("--dynamic-list=" + SanRT + ".syms"));
> return true;
> @@ -7014,7 +6943,7 @@
> }
> CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
>
> - addProfileRT(getToolChain(), Args, CmdArgs);
> + getToolChain().addProfileRTLibs(Args, CmdArgs);
>
> const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
> C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
> @@ -7605,7 +7534,7 @@
> CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
> }
>
> - addProfileRT(ToolChain, Args, CmdArgs);
> + ToolChain.addProfileRTLibs(Args, CmdArgs);
>
> const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
> C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
> @@ -7894,7 +7823,7 @@
> CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
> }
>
> - addProfileRT(getToolChain(), Args, CmdArgs);
> + getToolChain().addProfileRTLibs(Args, CmdArgs);
>
> const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
> C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
> @@ -8419,7 +8348,7 @@
> bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
> AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs);
> // The profile runtime also needs access to system libraries.
> - addProfileRT(getToolChain(), Args, CmdArgs);
> + getToolChain().addProfileRTLibs(Args, CmdArgs);
>
> if (D.CCCIsCXX() && !Args.hasArg(options::OPT_nostdlib) &&
> !Args.hasArg(options::OPT_nodefaultlibs)) {
> @@ -8730,7 +8659,7 @@
>
> AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
>
> - addProfileRT(getToolChain(), Args, CmdArgs);
> + getToolChain().addProfileRTLibs(Args, CmdArgs);
>
> if (!Args.hasArg(options::OPT_nostdlib) &&
> !Args.hasArg(options::OPT_nodefaultlibs)) {
> @@ -8922,7 +8851,7 @@
> CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
> }
>
> - addProfileRT(getToolChain(), Args, CmdArgs);
> + getToolChain().addProfileRTLibs(Args, CmdArgs);
>
> const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
> C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
> @@ -9028,18 +8957,18 @@
> "asan_dynamic", "asan_dynamic_runtime_thunk",
> };
> for (const auto &Component : CompilerRTComponents)
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, Component));
> + CmdArgs.push_back(TC.getCompilerRTArgString(Args, Component));
> // Make sure the dynamic runtime thunk is not optimized out at link time
> // to ensure proper SEH handling.
> CmdArgs.push_back(Args.MakeArgString("-include:___asan_seh_interceptor"));
> } else if (DLL) {
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, "asan_dll_thunk"));
> + CmdArgs.push_back(TC.getCompilerRTArgString(Args, "asan_dll_thunk"));
> } else {
> static const char *CompilerRTComponents[] = {
> "asan", "asan_cxx",
> };
> for (const auto &Component : CompilerRTComponents)
> - CmdArgs.push_back(getCompilerRTArgString(TC, Args, Component));
> + CmdArgs.push_back(TC.getCompilerRTArgString(Args, Component));
> }
> }
>
> Index: lib/Driver/ToolChains.h
> ===================================================================
> --- lib/Driver/ToolChains.h
> +++ lib/Driver/ToolChains.h
> @@ -275,9 +275,11 @@
>
> /// Add any profiling runtime libraries that are needed. This is essentially a
> /// MachO specific version of addProfileRT in Tools.cpp.
> - virtual void addProfileRTLibs(const llvm::opt::ArgList &Args,
> - llvm::opt::ArgStringList &CmdArgs) const {
> + virtual bool addProfileRTLibs(
> + const llvm::opt::ArgList &Args,
> + llvm::opt::ArgStringList &CmdArgs) const override {
LLVM style usually omits `virtual` when `override` is present (it's
redundant).
Also, I think nobody ended up using the bool result - so we might as
well just leave it as void for now.
> // There aren't any profiling libs for embedded targets currently.
> + return false;
> }
>
> /// }
> @@ -378,7 +380,7 @@
> return !isTargetIPhoneOS() || isIPhoneOSVersionLT(6, 0);
> }
>
> - void addProfileRTLibs(const llvm::opt::ArgList &Args,
> + bool addProfileRTLibs(const llvm::opt::ArgList &Args,
> llvm::opt::ArgStringList &CmdArgs) const override;
>
> protected:
> Index: lib/Driver/ToolChains.cpp
> ===================================================================
> --- lib/Driver/ToolChains.cpp
> +++ lib/Driver/ToolChains.cpp
> @@ -298,7 +298,7 @@
> }
> }
>
> -void Darwin::addProfileRTLibs(const ArgList &Args,
> +bool Darwin::addProfileRTLibs(const ArgList &Args,
> ArgStringList &CmdArgs) const {
> if (!(Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
> false) ||
> @@ -308,15 +308,16 @@
> Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> Args.hasArg(options::OPT_fcreate_profile) ||
> Args.hasArg(options::OPT_coverage)))
> - return;
> + return false;
>
> // Select the appropriate runtime library for the target.
> if (isTargetIOSBased())
> AddLinkRuntimeLib(Args, CmdArgs, "libclang_rt.profile_ios.a",
> /*AlwaysLink*/ true);
> else
> AddLinkRuntimeLib(Args, CmdArgs, "libclang_rt.profile_osx.a",
> /*AlwaysLink*/ true);
> + return true;
> }
>
> void DarwinClang::AddLinkSanitizerLibArgs(const ArgList &Args,
> Index: lib/Driver/ToolChain.cpp
> ===================================================================
> --- lib/Driver/ToolChain.cpp
> +++ lib/Driver/ToolChain.cpp
> @@ -456,6 +456,78 @@
>
> void ToolChain::addClangWarningOptions(ArgStringList &CC1Args) const {}
>
> +bool ToolChain::addProfileRTLibs(const llvm::opt::ArgList &Args,
> + llvm::opt::ArgStringList &CmdArgs) const {
> + if (!(Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
> + false) ||
> + Args.hasArg(options::OPT_fprofile_generate) ||
> + Args.hasArg(options::OPT_fprofile_generate_EQ) ||
> + Args.hasArg(options::OPT_fprofile_instr_generate) ||
> + Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
> + Args.hasArg(options::OPT_fcreate_profile) ||
> + Args.hasArg(options::OPT_coverage)))
> + return false;
> +
> + CmdArgs.push_back(getCompilerRTArgString(Args, "profile"));
> + return true;
> +}
> +
> +StringRef ToolChain::getArchNameForCompilerRTLib(
> + const llvm::opt::ArgList &Args) const {
> + const llvm::Triple &Triple = getTriple();
> + bool IsWindows = Triple.isOSWindows();
> +
> + if (Triple.isWindowsMSVCEnvironment() && getArch() == llvm::Triple::x86)
> + return "i386";
> +
> + if (getArch() == llvm::Triple::arm || getArch() == llvm::Triple::armeb)
> + return (tools::arm::getARMFloatABI(*this, Args) ==
> + tools::arm::FloatABI::Hard &&
> + !IsWindows)
> + ? "armhf"
> + : "arm";
> +
> + return getArchName();
> +}
> +
> +SmallString<128> ToolChain::getCompilerRTLibDir() const {
> + // The runtimes are located in the OS-specific resource directory.
> + SmallString<128> Res(getDriver().ResourceDir);
> + const llvm::Triple &Triple = getTriple();
> + // TC.getOS() yield "freebsd10.0" whereas "freebsd" is expected.
> + StringRef OSLibName =
> + (Triple.getOS() == llvm::Triple::FreeBSD) ? "freebsd" : getOS();
> + llvm::sys::path::append(Res, "lib", OSLibName);
> + return Res;
> +}
> +
> +SmallString<128> ToolChain::getCompilerRT(const llvm::opt::ArgList &Args,
> + StringRef Component,
> + bool Shared) const {
> + const char *Env =
> + getTriple().getEnvironment() == llvm::Triple::Android ? "-android" : "";
> +
> + bool IsOSWindows = getTriple().isOSWindows();
> + bool IsITANMSVCWindows = getTriple().isWindowsMSVCEnvironment() ||
> + getTriple().isWindowsItaniumEnvironment();
> + StringRef Arch = getArchNameForCompilerRTLib(Args);
> + const char *Prefix = IsITANMSVCWindows ? "" : "lib";
> + const char *Suffix = Shared ? (IsOSWindows ? ".dll" : ".so")
> + : (IsITANMSVCWindows ? ".lib" : ".a");
> +
> + SmallString<128> Path = getCompilerRTLibDir();
> + llvm::sys::path::append(Path, Prefix + Twine("clang_rt.") + Component + "-" +
> + Arch + Env + Suffix);
> +
> + return Path;
> +}
> +
> +const char *ToolChain::getCompilerRTArgString(const llvm::opt::ArgList &Args,
> + StringRef Component,
> + bool Shared) const {
> + return Args.MakeArgString(getCompilerRT(Args, Component, Shared));
> +}
> +
> ToolChain::RuntimeLibType ToolChain::GetRuntimeLibType(
> const ArgList &Args) const
> {
> Index: lib/Driver/SanitizerArgs.cpp
> ===================================================================
> --- lib/Driver/SanitizerArgs.cpp
> +++ lib/Driver/SanitizerArgs.cpp
> @@ -609,13 +609,11 @@
> if (TC.getTriple().isOSWindows() && needsUbsanRt()) {
> // Instruct the code generator to embed linker directives in the object file
> // that cause the required runtime libraries to be linked.
> - CmdArgs.push_back(
> - Args.MakeArgString("--dependent-lib=" +
> - tools::getCompilerRT(TC, Args, "ubsan_standalone")));
> + CmdArgs.push_back(Args.MakeArgString(
> + "--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone")));
> if (types::isCXX(InputType))
> CmdArgs.push_back(Args.MakeArgString(
> - "--dependent-lib=" +
> - tools::getCompilerRT(TC, Args, "ubsan_standalone_cxx")));
> + "--dependent-lib=" + TC.getCompilerRT(Args, "ubsan_standalone_cxx")));
> }
> }
>
> Index: include/clang/Driver/ToolChain.h
> ===================================================================
> --- include/clang/Driver/ToolChain.h
> +++ include/clang/Driver/ToolChain.h
> @@ -96,6 +96,8 @@
> virtual Tool *buildAssembler() const;
> virtual Tool *buildLinker() const;
> virtual Tool *getTool(Action::ActionClass AC) const;
> + SmallString<128> getCompilerRTLibDir() const;
> + StringRef getArchNameForCompilerRTLib(const llvm::opt::ArgList &Args) const;
>
> /// \name Utilities for implementing subclasses.
> ///@{
> @@ -165,6 +167,13 @@
> static std::pair<std::string, std::string>
> getTargetAndModeFromProgramName(StringRef ProgName);
>
> + /// Return link commmand line arguments for clang runtime libraries.
> + SmallString<128> getCompilerRT(const llvm::opt::ArgList &Args,
> + StringRef Component,
> + bool Shared = false) const;
> + const char *getCompilerRTArgString(const llvm::opt::ArgList &Args,
> + StringRef Component,
> + bool Shared = false) const;
> // Tool access.
>
> /// TranslateArgs - Create a new derived argument list for any argument
> @@ -364,6 +373,11 @@
> AddFastMathRuntimeIfAvailable(const llvm::opt::ArgList &Args,
> llvm::opt::ArgStringList &CmdArgs) const;
>
> + /// addProfileRTLibs - When -fprofile-instr-profile is specified, add profile
> + /// runtime library, otherwise return false.
> + virtual bool addProfileRTLibs(const llvm::opt::ArgList &Args,
> + llvm::opt::ArgStringList &CmdArgs) const;
> +
> /// \brief Return sanitizers which are available in this toolchain.
> virtual SanitizerMask getSupportedSanitizers() const;
> };
More information about the cfe-commits
mailing list