[TLII] Reduce copies of TLII for TLA

[wenlei]

Summary

Minor changes to reduce the copying needed for TargetLibraryInfoImpl when it's passed around by TargetLibraryAnalysis and TargetLibraryInfoWrapperPass, by using move whenever possible.

[hoyFB]

Thanks for the nice cleanup!

llvm/include/llvm/Analysis/TargetLibraryInfo.h

456

Can T just be used here? TargetLibraryInfoImpl comes with a such constructor: TargetLibraryInfoImpl(const Triple &T)

[hoyFB]

llvm/include/llvm/Analysis/TargetLibraryInfo.h

456

Unlikely not. That is an explicit constructor and BaselineInfoImpl is an Optional object.

[tejohnson]

Some parts of this are dependent on the patch that got reverted, but I have some other questions below about the changes in BackendUtil.cpp.

clang/lib/CodeGen/BackendUtil.cpp

691

These changes mean we now construct a new TLII multiple times (e.g. both when we add the TargetLibraryInfoWrapperPass to the MPM earlier and to the FPM here, rather that just copying. Is this actually faster? It seems like it would be slower overall.

llvm/lib/Analysis/TargetLibraryInfo.cpp

586–587

Woops, sorry, missed this one in the original review.

598

This is missing copying of the VecLibDescs (looks like I missed this as well originally).

610

Ditto.

[wenlei]

In D77952#1976336, @tejohnson wrote:

Some parts of this are dependent on the patch that got reverted, but I have some other questions below about the changes in BackendUtil.cpp.

Thanks for quick review.. I will remove the changes dependent on the reverted change before commit.

clang/lib/CodeGen/BackendUtil.cpp

691

Oops, this one isn't intentional... changed it back. Though for other instances where TLII isn't reused, similar change turns extra copy into move.

[wenlei]

address feedback

[tejohnson]

clang/lib/CodeGen/BackendUtil.cpp

691

I suppose you could std::move the TLII here to avoid this second copy.

Do you know how much difference this patch makes on the compile time instruction count regressions seen with the original patch? It seems like it shouldn't be huge given that this is just during the pipeline setup for the module. But if it does explain the instruction count increases that's probably why it didn't regress the actual wall time measurements.

[tejohnson]

Also, just a nit, because TLI is sometimes used to refer to the TargetLibraryInfo and occasionally to the TargetLibraryInfoImpl, and the latter is frequently referred to as TLII, could you change the description to say TLII or TLI Impl? Since this doesn't affect TargetLibraryInfo/TLI object copies.

[wenlei]

clang/lib/CodeGen/BackendUtil.cpp

691

Yeah, the 2nd use can be a move, though I'm inclined to not do that, as TLII isn't immediately out of scope yet.

I didn't setup measurement for this. I was basically playing with some tests for sanity check just to make sure we're not doing things unexpectedly, e.g. we don't do per-function copies unexpectedly in TargetLibraryAnalysis::run. But other than a few extra copies on setup path, I didn't see anything unusual. Since the original patch can make any copies more expensive, I thought it's good to reduce that as I see it.

llvm/lib/Analysis/TargetLibraryInfo.cpp

598

Now I remembered why this was missed from my side, before my patch, VecLibDescs isn't copied here either, which seems like a bug? Same for the move one below.

[wenlei]

In D77952#1976803, @tejohnson wrote:

Also, just a nit, because TLI is sometimes used to refer to the TargetLibraryInfo and occasionally to the TargetLibraryInfoImpl, and the latter is frequently referred to as TLII, could you change the description to say TLII or TLI Impl? Since this doesn't affect TargetLibraryInfo/TLI object copies.

Sure, done.. changed the title, and made the description more explicit.

[tejohnson]

lgtm with the VecLibDesc references removed.

llvm/lib/Analysis/TargetLibraryInfo.cpp

598

I assume you mean the VectorDescs/ScalarDescs vectors. You are right, even before your D77632 patch they were not being copied here or below. Looks like there is an existing bug there that no one noticed.

clang/lib/CodeGen/BackendUtil.cpp

static void addDataFlowSanitizerPass(const PassManagerBuilder &Builder,
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
 static void addMemTagOptimizationPasses(const PassManagerBuilder &Builder,
                                         legacy::PassManagerBase &PM) {
   PM.add(createStackSafetyGlobalInfoWrapperPass(/*SetMetadata=*/true));
 }
 
-static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple) {
-  return new TargetLibraryInfoImpl(TargetTriple);
-}
-
 static void addSymbolRewriterPass(const CodeGenOptions &Opts,
                                   legacy::PassManager *MPM) {
   llvm::SymbolRewriter::RewriteDescriptorList DL;
 
   llvm::SymbolRewriter::RewriteMapParser MapParser;
   for (const auto &MapFile : Opts.RewriteMapFiles)
     MapParser.parse(MapFile, &DL);
 

 
 void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
                                       legacy::FunctionPassManager &FPM) {
   // Handle disabling of all LLVM passes, where we want to preserve the
   // internal module before any optimization.
   if (CodeGenOpts.DisableLLVMPasses)
     return;
 
-  // Figure out TargetLibraryInfo.  This needs to be added to MPM and FPM
+  // Figure out TargetLibraryInfo. This needs to be added to MPM and FPM
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
   Triple TargetTriple(TheModule->getTargetTriple());
-  std::unique_ptr<TargetLibraryInfoImpl> TLII(createTLII(TargetTriple));
+  TargetLibraryInfoImpl TLII(TargetTriple);
 
   // If we reached here with a non-empty index file name, then the index file
   // was empty and we are not performing ThinLTO backend compilation (used in
   // testing in a distributed build environment). Drop any the type test
   // assume sequences inserted for whole program vtables so that codegen doesn't
   // complain.
   if (!CodeGenOpts.ThinLTOIndexFile.empty())
     MPM.add(createLowerTypeTestsPass(/*ExportSummary=*/nullptr,
void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
   // Loop interleaving in the loop vectorizer has historically been set to be
   // enabled when loop unrolling is enabled.
   PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
   PMBuilder.RerollLoops = CodeGenOpts.RerollLoops;
 
-  MPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  MPM.add(new TargetLibraryInfoWrapperPass(TLII));
 
   if (TM)
     TM->adjustPassManager(PMBuilder);
 
   if (CodeGenOpts.DebugInfoForProfiling ||
       !CodeGenOpts.SampleProfileFile.empty())
     PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
                            addAddDiscriminatorsPass);
void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
   }
 
   if (LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
     PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
                            addMemTagOptimizationPasses);
   }
 
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  FPM.add(new TargetLibraryInfoWrapperPass(TLII));
   if (CodeGenOpts.VerifyModule)
     FPM.add(createVerifierPass());
 
   // Set up the per-module pass manager.
   if (!CodeGenOpts.RewriteMapFiles.empty())
     addSymbolRewriterPass(CodeGenOpts, &MPM);
 
   if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts)) {

 }
 
 bool EmitAssemblyHelper::AddEmitPasses(legacy::PassManager &CodeGenPasses,
                                        BackendAction Action,
                                        raw_pwrite_stream &OS,
                                        raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
   llvm::Triple TargetTriple(TheModule->getTargetTriple());
-  std::unique_ptr<TargetLibraryInfoImpl> TLII(createTLII(TargetTriple));
-  CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
+  CodeGenPasses.add(new TargetLibraryInfoWrapperPass(TargetTriple));
 
   // Normal mode, emit a .s or .o file by running the code generator. Note,
   // this also adds codegenerator level optimization passes.
   CodeGenFileType CGFT = getCodeGenFileType(Action);
 
   // Add ObjC ARC final-cleanup optimizations. This is done as part of the
   // "codegen" passes so that it isn't run multiple times when there is
   // inlining happening.
#include "llvm/Support/Extension.def"
   ModuleAnalysisManager MAM(CodeGenOpts.DebugPassManager);
 
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return PB.buildDefaultAAPipeline(); });
 
   // Register the target library analysis directly and give it a customized
   // preset TLI.
   Triple TargetTriple(TheModule->getTargetTriple());
-  std::unique_ptr<TargetLibraryInfoImpl> TLII(createTLII(TargetTriple));
-  FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
+  FAM.registerPass([&] { return TargetLibraryAnalysis(TargetTriple); });
 
   // Register all the basic analyses with the managers.
   PB.registerModuleAnalyses(MAM);
   PB.registerCGSCCAnalyses(CGAM);
   PB.registerFunctionAnalyses(FAM);
   PB.registerLoopAnalyses(LAM);
   PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
 

llvm/include/llvm/Analysis/TargetLibraryInfo.h

public:
   ///
   /// This will use the module's triple to construct the library info for that
   /// module.
   TargetLibraryAnalysis() {}
 
   /// Construct a library analysis with baseline Module-level info.
   ///
   /// This will be supplemented with Function-specific info in the Result.
-  TargetLibraryAnalysis(TargetLibraryInfoImpl BaselineInfoImpl)
+  TargetLibraryAnalysis(const Triple &T)
+      : BaselineInfoImpl(TargetLibraryInfoImpl(T)) {}
+  TargetLibraryAnalysis(const TargetLibraryInfoImpl &BaselineInfoImpl)
+      : BaselineInfoImpl(BaselineInfoImpl) {}
+  TargetLibraryAnalysis(TargetLibraryInfoImpl &&BaselineInfoImpl)
       : BaselineInfoImpl(std::move(BaselineInfoImpl)) {}
 
   TargetLibraryInfo run(const Function &F, FunctionAnalysisManager &);
 
 private:
   friend AnalysisInfoMixin<TargetLibraryAnalysis>;
   static AnalysisKey Key;
 
   Optional<TargetLibraryInfoImpl> BaselineInfoImpl;
 };
 
 class TargetLibraryInfoWrapperPass : public ImmutablePass {
   TargetLibraryAnalysis TLA;
   Optional<TargetLibraryInfo> TLI;
 
   virtual void anchor();
 
 public:
   static char ID;
   TargetLibraryInfoWrapperPass();
   explicit TargetLibraryInfoWrapperPass(const Triple &T);
   explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfoImpl &TLI);
+  explicit TargetLibraryInfoWrapperPass(TargetLibraryInfoImpl &&TLI);
 
   TargetLibraryInfo &getTLI(const Function &F) {
     FunctionAnalysisManager DummyFAM;
     TLI = TLA.run(F, DummyFAM);
     return *TLI;
   }
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Analysis/TargetLibraryInfo.cpp

 
 TargetLibraryInfoImpl::TargetLibraryInfoImpl(TargetLibraryInfoImpl &&TLI)
     : CustomNames(std::move(TLI.CustomNames)),
       ShouldExtI32Param(TLI.ShouldExtI32Param),
       ShouldExtI32Return(TLI.ShouldExtI32Return),
       ShouldSignExtI32Param(TLI.ShouldSignExtI32Param) {
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
             AvailableArray);
-  for (unsigned i = 0; i < NumVecLibs; i++)
-    VecLibDescs[i] = TLI.VecLibDescs[i];
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+            VecLibDescs);
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const TargetLibraryInfoImpl &TLI) {
   CustomNames = TLI.CustomNames;
   ShouldExtI32Param = TLI.ShouldExtI32Param;
   ShouldExtI32Return = TLI.ShouldExtI32Return;
   ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
+  for (unsigned i = 0; i < NumVecLibs; i++)
+    VecLibDescs[i] = TLI.VecLibDescs[i];
   return *this;
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(TargetLibraryInfoImpl &&TLI) {
   CustomNames = std::move(TLI.CustomNames);
   ShouldExtI32Param = TLI.ShouldExtI32Param;
   ShouldExtI32Return = TLI.ShouldExtI32Return;
   ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
             AvailableArray);
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+            VecLibDescs);
   return *this;
 }
 
 static StringRef sanitizeFunctionName(StringRef funcName) {
   // Filter out empty names and names containing null bytes, those can't be in
   // our table.
   if (funcName.empty() || funcName.find('\0') != StringRef::npos)
     return StringRef();
 

 }
 
 TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
     const TargetLibraryInfoImpl &TLIImpl)
     : ImmutablePass(ID), TLA(TLIImpl) {
   initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
+TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
+    TargetLibraryInfoImpl &&TLIImpl)
+    : ImmutablePass(ID), TLA(std::move(TLIImpl)) {
+  initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
 AnalysisKey TargetLibraryAnalysis::Key;
 
 // Register the basic pass.
 INITIALIZE_PASS(TargetLibraryInfoWrapperPass, "targetlibinfo",
                 "Target Library Information", false, true)
 char TargetLibraryInfoWrapperPass::ID = 0;
 
 void TargetLibraryInfoWrapperPass::anchor() {}