[Vector] Pass VectLib to LTO backend so TLI build correct vector function list

[wenlei]

Summary

-fveclib switch not propagated to LTO backends, and as a result, LTO populates vector list as if no math lib is used. This change fixed the driver to lld, and to backend propagation of -fveclib setting.

[MaskRay]

Added usual lld and LTO side reviewers. I am just a bit worried that option name changes like this patch and D77231 could accidentally slip through if I did not react in time...

[tejohnson]

We're trying to move towards encoding all of this in the IR. And in fact, I recently implemented a series of patches to make the TLI to be built per-function, and along with some patches from @gchatelet to encode -fno-builtin* as function attributes, we now handle that part of the TLII with IR. See D67923 which is the last patch in the series. We should encode the vectlib as function attributes similarly, and just thread that through to the TLI.

[wenlei]

In D77484#1962445, @tejohnson wrote:

We're trying to move towards encoding all of this in the IR. And in fact, I recently implemented a series of patches to make the TLI to be built per-function, and along with some patches from @gchatelet to encode -fno-builtin* as function attributes, we now handle that part of the TLII with IR. See D67923 which is the last patch in the series. We should encode the vectlib as function attributes similarly, and just thread that through to the TLI.

Thanks for quick look, @tejohnson. Yeah, making vectlib a function attribute would be more consistent with other the way other TLI attributes are handled. However, unlike some other attributes, which really need to be per-function, conceptually, vectlib setting won't be different from function to function in a module. Implementation-wise, even though TLI is now built per-function, there's a BaselineInfoImpl within each TargetLibraryInfo, that's still shared among functions, and VectorDescs which is populated based on vectlib setting belongs to the shared baseline. We could also move VectorDescs and its population into per-function part of TLI, but I felt that's a bit overkill, though I don't have a strong opinion on this. So I followed how other module level stuff like lto-sample-profile is passed to backend..

Alternatively, we could chose to always run InjectTLIMappings in pre-LTO pipeline, even if we don't run vectorizer pre-LTO for ThinLTO, so we always have per-function attribute vector-function-abi-variant populated and threaded through to LTO time vectorizer. We will have to make sure the vector function list in TLI (based on vectlib setting) has no other use case though.

(More on TLI, while this change (or a variant of this) enables vectorization that uses math library for ThinLTO w/ legacy PM, there's separate issue with new PM as InjectTLIMappings is not scheduled before Vectorization for LTO pipeline. I will send a separate patch for that once we settle on a good way to handle this one.)

[hoyFB]

In D77484#1962445, @tejohnson wrote:

We're trying to move towards encoding all of this in the IR. And in fact, I recently implemented a series of patches to make the TLI to be built per-function, and along with some patches from @gchatelet to encode -fno-builtin* as function attributes, we now handle that part of the TLII with IR. See D67923 which is the last patch in the series. We should encode the vectlib as function attributes similarly, and just thread that through to the TLI.

That's an interesting idea. How does the linkage work if two functions have different vectlib attributes? Linking against two vectlibs may cause name conflicts or other issues.

[tejohnson]

In D77484#1962581, @hoyFB wrote:

In D77484#1962445, @tejohnson wrote:

We're trying to move towards encoding all of this in the IR. And in fact, I recently implemented a series of patches to make the TLI to be built per-function, and along with some patches from @gchatelet to encode -fno-builtin* as function attributes, we now handle that part of the TLII with IR. See D67923 which is the last patch in the series. We should encode the vectlib as function attributes similarly, and just thread that through to the TLI.

That's an interesting idea. How does the linkage work if two functions have different vectlib attributes? Linking against two vectlibs may cause name conflicts or other issues.

Actually this may be best encoded in a module metadata that with an error merging type, so that conflicting options get flagged. What happens today in a non-LTO build if conflicting veclib options are provided to different TUs?

[wenlei]

Linking against two vectlibs may cause name conflicts or other issues.

Of all three supported match libraries, all functions from Accelerate are prefixed with v; all MASS library functions are suffixed with _massv; and all SVML functions are prefixed with __svml_. See VecFuncs.def. So at least the exported functions don't have name conflicts.

What happens today in a non-LTO build if conflicting veclib options are provided to different TUs?

I think it will work, as long as used math libraries are all provided to linker. Even if not, I'm not sure if this is something we want to actively prevent from the use of fvectlib switch. fvectlib controls codegen/optimizer, and whether the resulting codegen can leads to linking problem (if any), is kind of orthogonal, and probably no different from a regular linking/symbol resolution problem if there're conflicts in the libraries provide.

[tejohnson]

In D77484#1962667, @wenlei wrote:

Linking against two vectlibs may cause name conflicts or other issues.

Of all three supported match libraries, all functions from Accelerate are prefixed with v; all MASS library functions are suffixed with _massv; and all SVML functions are prefixed with __svml_. See VecFuncs.def. So at least the exported functions don't have name conflicts.

Ok then it does sound like these could be handled on a per-function basis, similar to how -fno-builtin* are handled. I.e. a function attribute to indicate the veclib, which would then be naturally preserved during LTO even after merging/importing across modules. Similar to how -fno-builtin* are handled, these would need to be examined when inlining (see the new TargetLibraryInfo::areInlineCompatible). Presumably we would want to block inlining between functions with different veclib attributes in the LTO backends.

What happens today in a non-LTO build if conflicting veclib options are provided to different TUs?

I think it will work, as long as used math libraries are all provided to linker. Even if not, I'm not sure if this is something we want to actively prevent from the use of fvectlib switch. fvectlib controls codegen/optimizer, and whether the resulting codegen can leads to linking problem (if any), is kind of orthogonal, and probably no different from a regular linking/symbol resolution problem if there're conflicts in the libraries provide.

[ruiu]

lld/ELF/Config.h

132

We name variables after their corresponding command line flags, so this should be ltoVectorLibrary.

[grimar]

lld/ELF/Options.td

491

You need to update the documentation either (lld/docs/ld.lld.1)

[wenlei]

Ok then it does sound like these could be handled on a per-function basis, similar to how -fno-builtin* are handled. I.e. a function attribute to indicate the veclib, which would then be naturally preserved during LTO even after merging/importing across modules. Similar to how -fno-builtin* are handled, these would need to be examined when inlining (see the new TargetLibraryInfo::areInlineCompatible). Presumably we would want to block inlining between functions with different veclib attributes in the LTO backends.

@tejohnson, we could do that. But then on the other hand, technically almost everything for module or whole program can be passed as a function attribute, and yet we have switches passed to backend for many of those things. Wondering what's the convention or rule (if there's one) we want to follow? Specifically, if we only use function attributes for stuff that's indeed going to be different between functions, then vectlib isn't in that category; or if we use function attributes for the greatest flexibility whenever we can, then many other things should be function attributes too (though it's essentially duplication in IR, and probably not the most efficient).

[tejohnson]

In D77484#1965629, @wenlei wrote:

Ok then it does sound like these could be handled on a per-function basis, similar to how -fno-builtin* are handled. I.e. a function attribute to indicate the veclib, which would then be naturally preserved during LTO even after merging/importing across modules. Similar to how -fno-builtin* are handled, these would need to be examined when inlining (see the new TargetLibraryInfo::areInlineCompatible). Presumably we would want to block inlining between functions with different veclib attributes in the LTO backends.

@tejohnson, we could do that. But then on the other hand, technically almost everything for module or whole program can be passed as a function attribute, and yet we have switches passed to backend for many of those things. Wondering what's the convention or rule (if there's one) we want to follow? Specifically, if we only use function attributes for stuff that's indeed going to be different between functions, then vectlib isn't in that category; or if we use function attributes for the greatest flexibility whenever we can, then many other things should be function attributes too (though it's essentially duplication in IR, and probably not the most efficient).

Passing the option through the driver to the linker is the legacy approach. But it isn't really scalable and has other issues, so we've been moving towards having all the necessary info in the IR itself. For one, this helps deal with cases where different options were specified for different source files. For another, it keeps the same build behavior with LTO and non-LTO. I.e. for this option, if the build system specified it for the cc compiles but not the links, it would work for O2 but not for LTO if it had to be propagated via the linker. It would work for LTO if it was propagated via the IR.

[wenlei]

In D77484#1965976, @tejohnson wrote:

In D77484#1965629, @wenlei wrote:

Ok then it does sound like these could be handled on a per-function basis, similar to how -fno-builtin* are handled. I.e. a function attribute to indicate the veclib, which would then be naturally preserved during LTO even after merging/importing across modules. Similar to how -fno-builtin* are handled, these would need to be examined when inlining (see the new TargetLibraryInfo::areInlineCompatible). Presumably we would want to block inlining between functions with different veclib attributes in the LTO backends.

@tejohnson, we could do that. But then on the other hand, technically almost everything for module or whole program can be passed as a function attribute, and yet we have switches passed to backend for many of those things. Wondering what's the convention or rule (if there's one) we want to follow? Specifically, if we only use function attributes for stuff that's indeed going to be different between functions, then vectlib isn't in that category; or if we use function attributes for the greatest flexibility whenever we can, then many other things should be function attributes too (though it's essentially duplication in IR, and probably not the most efficient).

Passing the option through the driver to the linker is the legacy approach. But it isn't really scalable and has other issues, so we've been moving towards having all the necessary info in the IR itself. For one, this helps deal with cases where different options were specified for different source files. For another, it keeps the same build behavior with LTO and non-LTO. I.e. for this option, if the build system specified it for the cc compiles but not the links, it would work for O2 but not for LTO if it had to be propagated via the linker. It would work for LTO if it was propagated via the IR.

Makes sense, thanks for clarification. I created D77632 to make vect lib setting a per-function attribute.

clang/lib/Driver/ToolChains/CommonArgs.cpp

 //===--- CommonArgs.cpp - Args handling for multiple toolchains -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
#endif
   }
 
   // Setup statistics file output.
   SmallString<128> StatsFile =
       getStatsFileName(Args, Output, Input, ToolChain.getDriver());
   if (!StatsFile.empty())
     CmdArgs.push_back(
         Args.MakeArgString(Twine("-plugin-opt=stats-file=") + StatsFile));
+
+  // Pass vector library setting for LTO-time vectorization
+  if (Arg *A = Args.getLastArg(options::OPT_fveclib)) {
+    StringRef Name = A->getValue();
+    if (!Name.empty())
+      CmdArgs.push_back(
+          Args.MakeArgString(Twine("-plugin-opt=vector-library=") + Name));
+  }
 }
 
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
                                  ArgStringList &CmdArgs) {
   if (!Args.hasFlag(options::OPT_frtlib_add_rpath,
                     options::OPT_fno_rtlib_add_rpath, false))
     return;
 

lld/ELF/Config.h

 //===- Config.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_ELF_CONFIG_H
 #define LLD_ELF_CONFIG_H
 
 #include "lld/Common/ErrorHandler.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Endian.h"
 #include <atomic>
 #include <vector>
 
 namespace lld {
struct Configuration {
   std::vector<llvm::StringRef> searchPaths;
   std::vector<llvm::StringRef> symbolOrderingFile;
   std::vector<llvm::StringRef> undefined;
   std::vector<SymbolVersion> dynamicList;
   std::vector<uint8_t> buildIdVector;
   llvm::MapVector<std::pair<const InputSectionBase *, const InputSectionBase *>,
                   uint64_t>
       callGraphProfile;
+  llvm::TargetLibraryInfoImpl::VectorLibrary VectLib;
   bool allowMultipleDefinition;
   bool allowShlibUndefined;
   bool androidPackDynRelocs;
   bool armHasBlx = false;
   bool armHasMovtMovw = false;
   bool armJ1J2BranchEncoding = false;
   bool asNeeded = false;
   bool bsymbolic;

lld/ELF/Driver.cpp

 //===- Driver.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
if (s == "warn")
     return OrphanHandlingPolicy::Warn;
   if (s == "error")
     return OrphanHandlingPolicy::Error;
   if (s != "place")
     error("unknown --orphan-handling mode: " + s);
   return OrphanHandlingPolicy::Place;
 }
 
+static llvm::TargetLibraryInfoImpl::VectorLibrary
+getVectLib(opt::InputArgList &args) {
+  StringRef s = args.getLastArgValue(OPT_lto_vector_library);
+  if (s == "Accelerate")
+    return llvm::TargetLibraryInfoImpl::Accelerate;
+  if (s == "MASSV")
+    return llvm::TargetLibraryInfoImpl::MASSV;
+  if (s == "SVML")
+    return llvm::TargetLibraryInfoImpl::SVML;
+  if (!s.empty() && s != "none")
+    error("unknown --vector-library type: " + s);
+  return llvm::TargetLibraryInfoImpl::NoLibrary;
+}
+
 // Parse --build-id or --build-id=<style>. We handle "tree" as a
 // synonym for "sha1" because all our hash functions including
 // -build-id=sha1 are actually tree hashes for performance reasons.
 static std::pair<BuildIdKind, std::vector<uint8_t>>
 getBuildId(opt::InputArgList &args) {
   auto *arg = args.getLastArg(OPT_build_id, OPT_build_id_eq);
   if (!arg)
     return {BuildIdKind::None, {}};
static void readConfigs(opt::InputArgList &args) {
   config->trace = args.hasArg(OPT_trace);
   config->undefined = args::getStrings(args, OPT_undefined);
   config->undefinedVersion =
       args.hasFlag(OPT_undefined_version, OPT_no_undefined_version, true);
   config->unique = args.hasArg(OPT_unique);
   config->useAndroidRelrTags = args.hasFlag(
       OPT_use_android_relr_tags, OPT_no_use_android_relr_tags, false);
   config->unresolvedSymbols = getUnresolvedSymbolPolicy(args);
+  config->VectLib = getVectLib(args);
   config->warnBackrefs =
       args.hasFlag(OPT_warn_backrefs, OPT_no_warn_backrefs, false);
   config->warnCommon = args.hasFlag(OPT_warn_common, OPT_no_warn_common, false);
   config->warnIfuncTextrel =
       args.hasFlag(OPT_warn_ifunc_textrel, OPT_no_warn_ifunc_textrel, false);
   config->warnSymbolOrdering =
       args.hasFlag(OPT_warn_symbol_ordering, OPT_no_warn_symbol_ordering, true);
   config->zCombreloc = getZFlag(args, "combreloc", "nocombreloc", true);

lld/ELF/LTO.cpp

 //===- LTO.cpp ------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
static lto::Config createConfig() {
   c.RemarksPasses = std::string(config->optRemarksPasses);
   c.RemarksWithHotness = config->optRemarksWithHotness;
   c.RemarksFormat = std::string(config->optRemarksFormat);
 
   c.SampleProfile = std::string(config->ltoSampleProfile);
   c.UseNewPM = config->ltoNewPassManager;
   c.DebugPassManager = config->ltoDebugPassManager;
   c.DwoDir = std::string(config->dwoDir);
+  c.Options.VectLib = config->VectLib;
 
   c.HasWholeProgramVisibility = config->ltoWholeProgramVisibility;
 
   c.TimeTraceEnabled = config->timeTraceEnabled;
   c.TimeTraceGranularity = config->timeTraceGranularity;
 
   c.CSIRProfile = std::string(config->ltoCSProfileFile);
   c.RunCSIRInstr = config->ltoCSProfileGenerate;

lld/ELF/Options.td

def lto_partitions: J<"lto-partitions=">,
   HelpText<"Number of LTO codegen partitions">;
 def lto_cs_profile_generate: F<"lto-cs-profile-generate">,
   HelpText<"Perform context sensitive PGO instrumentation">;
 def lto_cs_profile_file: J<"lto-cs-profile-file=">,
   HelpText<"Context sensitive profile file path">;
 def lto_obj_path_eq: J<"lto-obj-path=">;
 def lto_sample_profile: J<"lto-sample-profile=">,
   HelpText<"Sample profile file path">;
+def lto_vector_library: J<"lto-vector-library=">,
+  HelpText<"Vector functions library">;
 def lto_whole_program_visibility: F<"lto-whole-program-visibility">,
   HelpText<"Asserts that the LTO link has whole program visibility">;
 def disable_verify: F<"disable-verify">;
 defm mllvm: Eq<"mllvm", "Additional arguments to forward to LLVM's option processing">;
 def opt_remarks_filename: Separate<["--"], "opt-remarks-filename">,
   HelpText<"YAML output file for optimization remarks">;
 def opt_remarks_passes: Separate<["--"], "opt-remarks-passes">,
   HelpText<"Regex for the passes that need to be serialized to the output file">;
def: J<"plugin-opt=thinlto-index-only=">,
   Alias<thinlto_index_only_eq>,
   HelpText<"Alias for --thinlto-index-only=">;
 def: J<"plugin-opt=thinlto-object-suffix-replace=">,
  Alias<thinlto_object_suffix_replace_eq>,
  HelpText<"Alias for --thinlto-object-suffix-replace=">;
 def: J<"plugin-opt=thinlto-prefix-replace=">,
   Alias<thinlto_prefix_replace_eq>,
   HelpText<"Alias for --thinlto-prefix-replace=">;
+def: J<"plugin-opt=vector-library=">,
+  Alias<lto_vector_library>,
+  HelpText<"Alias for -lto-vector-library=">;
 
 // Ignore LTO plugin-related options.
 // clang -flto passes -plugin and -plugin-opt to the linker. This is required
 // for ld.gold and ld.bfd to get LTO working. But it's not for lld which doesn't
 // rely on a plugin. Instead of detecting which linker is used on clang side we
 // just ignore the option on lld side as it's easier. In fact, the linker could
 // be called 'ld' and understanding which linker is used would require parsing of
 // --version output.

llvm/include/llvm/Target/TargetOptions.h

 //===-- llvm/Target/TargetOptions.h - Target Options ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines command line option flags that are shared across various
 // targets.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TARGET_TARGETOPTIONS_H
 #define LLVM_TARGET_TARGETOPTIONS_H
 
 #include "llvm/ADT/FloatingPointMode.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 
 #include <memory>
 
 namespace llvm {
   struct fltSemantics;
   class MachineFunction;
   class MemoryBuffer;
public:
     /// precision won't effect the result.
     ///
     /// Note: This option only controls formation of fused ops by the
     /// optimizers.  Fused operations that are explicitly specified (e.g. FMA
     /// via the llvm.fma.* intrinsic) will always be honored, regardless of
     /// the value of this option.
     FPOpFusion::FPOpFusionMode AllowFPOpFusion = FPOpFusion::Standard;
 
+    /// Vector library used for vectorizable math function calls
+    TargetLibraryInfoImpl::VectorLibrary VectLib =
+        TargetLibraryInfoImpl::NoLibrary;
+
     /// ThreadModel - This flag specifies the type of threading model to assume
     /// for things like atomics
     ThreadModel::Model ThreadModel = ThreadModel::POSIX;
 
     /// EABIVersion - This flag specifies the EABI version
     EABI EABIVersion = EABI::Default;
 
     /// Which debugger to tune for.

llvm/lib/LTO/LTOBackend.cpp

 //===-LTOBackend.cpp - LLVM Link Time Optimizer Backend -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 static void runOldPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
                            bool IsThinLTO, ModuleSummaryIndex *ExportSummary,
                            const ModuleSummaryIndex *ImportSummary) {
   legacy::PassManager passes;
   passes.add(createTargetTransformInfoWrapperPass(TM->getTargetIRAnalysis()));
 
   PassManagerBuilder PMB;
   PMB.LibraryInfo = new TargetLibraryInfoImpl(Triple(TM->getTargetTriple()));
+  PMB.LibraryInfo->addVectorizableFunctionsFromVecLib(Conf.Options.VectLib);
   PMB.Inliner = createFunctionInliningPass();
   PMB.ExportSummary = ExportSummary;
   PMB.ImportSummary = ImportSummary;
   // Unconditionally verify input since it is not verified before this
   // point and has unknown origin.
   PMB.VerifyInput = true;
   PMB.VerifyOutput = !Conf.DisableVerify;
   PMB.LoopVectorize = true;