[PATCH] D13326: [PGO]: Eliminate __llvm_profile_register calls for Linux (clang changes)
Xinliang David Li via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 6 11:41:11 PDT 2015
Clang FE refactoring change is not needed anymore.
In commit 0d32e7d952bc80830183e0c2c6ec5027ca6b1450, Vasileios
Kalintiris did the same thing for multi-lib support.
David
On Tue, Oct 6, 2015 at 12:13 AM, Justin Bogner <mail at justinbogner.com> wrote:
> 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