[llvm] 6aa9cfb - [NVPTX] Replace PTX's ManagedStringPool with StringSaver

Luke Drummond via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 03:28:44 PST 2023


Author: Luke Drummond
Date: 2023-01-04T11:28:39Z
New Revision: 6aa9cfb13f2599d69f93941f338134fbcc80a799

URL: https://github.com/llvm/llvm-project/commit/6aa9cfb13f2599d69f93941f338134fbcc80a799
DIFF: https://github.com/llvm/llvm-project/commit/6aa9cfb13f2599d69f93941f338134fbcc80a799.diff

LOG: [NVPTX] Replace PTX's ManagedStringPool with StringSaver

In use ManagedStringPool caused a lot of heap allocations. At least one
for every register name lookup in NVPTXTargetRegisterInfo and one for
every symbol lookup in the target machine and isel lowering. There
already exists an llvm/Support string interning-class that has better
memory performance. Use LLVM's and delete ManagedStringPool which was
unique to PTX

llc Binary Size (.text only; bss and data were unchanged):
  MinsizeRel:
    Before: 31219884
    After: 31219796
  Release:
    Before: 42961872
    After: 42960656

Total heap allocations by the NVPTX string saving code running
check-llvm-codegen-nvptx

Total bytes allocated:
  Before: 2431825
  After: 2288151

(All numbers on x86-64-linux-gnu / gcc-12 / lld14)

I didn't see obvious time differences when running the tests.

Reviewers: tra, avasonic
Differential Revision: https://reviews.llvm.org/D140704

Added: 
    

Modified: 
    llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
    llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
    llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp
    llvm/lib/Target/NVPTX/NVPTXRegisterInfo.h
    llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
    llvm/lib/Target/NVPTX/NVPTXTargetMachine.h

Removed: 
    llvm/lib/Target/NVPTX/ManagedStringPool.h


################################################################################
diff  --git a/llvm/lib/Target/NVPTX/ManagedStringPool.h b/llvm/lib/Target/NVPTX/ManagedStringPool.h
deleted file mode 100644
index fb838d1fcd7fa..0000000000000
--- a/llvm/lib/Target/NVPTX/ManagedStringPool.h
+++ /dev/null
@@ -1,48 +0,0 @@
-//===-- ManagedStringPool.h - Managed String Pool ---------------*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// The strings allocated from a managed string pool are owned by the string
-// pool and will be deleted together with the managed string pool.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIB_TARGET_NVPTX_MANAGEDSTRINGPOOL_H
-#define LLVM_LIB_TARGET_NVPTX_MANAGEDSTRINGPOOL_H
-
-#include "llvm/ADT/SmallVector.h"
-#include <string>
-
-namespace llvm {
-
-/// ManagedStringPool - The strings allocated from a managed string pool are
-/// owned by the string pool and will be deleted together with the managed
-/// string pool.
-class ManagedStringPool {
-  SmallVector<std::string *, 8> Pool;
-
-public:
-  ManagedStringPool() = default;
-
-  ~ManagedStringPool() {
-    SmallVectorImpl<std::string *>::iterator Current = Pool.begin();
-    while (Current != Pool.end()) {
-      delete *Current;
-      ++Current;
-    }
-  }
-
-  std::string *getManagedString(const char *S) {
-    std::string *Str = new std::string(S);
-    Pool.push_back(Str);
-    return Str;
-  }
-};
-
-} // end namespace llvm
-
-#endif // LLVM_LIB_TARGET_NVPTX_MANAGEDSTRINGPOOL_H

diff  --git a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
index 7d8d37a0f97e2..eab19547101a6 100644
--- a/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -206,9 +206,8 @@ void NVPTXAsmPrinter::lowerImageHandleSymbol(unsigned Index, MCOperand &MCOp) {
   NVPTXTargetMachine &nvTM = static_cast<NVPTXTargetMachine&>(TM);
   const NVPTXMachineFunctionInfo *MFI = MF->getInfo<NVPTXMachineFunctionInfo>();
   const char *Sym = MFI->getImageHandleSymbol(Index);
-  std::string *SymNamePtr =
-    nvTM.getManagedStrPool()->getManagedString(Sym);
-  MCOp = GetSymbolRef(OutContext.getOrCreateSymbol(StringRef(*SymNamePtr)));
+  StringRef SymName = nvTM.getStrPool().save(Sym);
+  MCOp = GetSymbolRef(OutContext.getOrCreateSymbol(SymName));
 }
 
 void NVPTXAsmPrinter::lowerToMCInst(const MachineInstr *MI, MCInst &OutMI) {

diff  --git a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
index 8b571811cdac9..b7e81ea596b44 100644
--- a/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -1822,10 +1822,11 @@ SDValue NVPTXTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
                       ->getAPIntValue()))
             : std::nullopt,
         *CB, UniqueCallSite);
-    const char *ProtoStr =
-      nvTM->getManagedStrPool()->getManagedString(Proto.c_str())->c_str();
+    const char *ProtoStr = nvTM->getStrPool().save(Proto).data();
     SDValue ProtoOps[] = {
-      Chain, DAG.getTargetExternalSymbol(ProtoStr, MVT::i32), InFlag,
+        Chain,
+        DAG.getTargetExternalSymbol(ProtoStr, MVT::i32),
+        InFlag,
     };
     Chain = DAG.getNode(NVPTXISD::CallPrototype, dl, ProtoVTs, ProtoOps);
     InFlag = Chain.getValue(1);
@@ -2634,9 +2635,9 @@ SDValue NVPTXTargetLowering::getParamSymbol(SelectionDAG &DAG, int idx,
   else
     ParamStr << "_param_" << idx;
 
-  std::string *SavedStr =
-    nvTM->getManagedStrPool()->getManagedString(ParamSym.c_str());
-  return DAG.getTargetExternalSymbol(SavedStr->c_str(), v);
+  StringRef SavedStr =
+    nvTM->getStrPool().save(ParamSym);
+  return DAG.getTargetExternalSymbol(SavedStr.data(), v);
 }
 
 SDValue NVPTXTargetLowering::LowerFormalArguments(

diff  --git a/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp b/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp
index 291572ba58efd..6e4208d272412 100644
--- a/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.cpp
@@ -93,7 +93,8 @@ std::string getNVPTXRegClassStr(TargetRegisterClass const *RC) {
 }
 }
 
-NVPTXRegisterInfo::NVPTXRegisterInfo() : NVPTXGenRegisterInfo(0) {}
+NVPTXRegisterInfo::NVPTXRegisterInfo()
+    : NVPTXGenRegisterInfo(0), StrPool(StrAlloc) {}
 
 #define GET_REGINFO_TARGET_DESC
 #include "NVPTXGenRegisterInfo.inc"

diff  --git a/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.h b/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.h
index 57218b9d60af8..7bce3bd18ae8f 100644
--- a/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.h
+++ b/llvm/lib/Target/NVPTX/NVPTXRegisterInfo.h
@@ -13,8 +13,8 @@
 #ifndef LLVM_LIB_TARGET_NVPTX_NVPTXREGISTERINFO_H
 #define LLVM_LIB_TARGET_NVPTX_NVPTXREGISTERINFO_H
 
-#include "ManagedStringPool.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
+#include "llvm/Support/StringSaver.h"
 #include <sstream>
 
 #define GET_REGINFO_HEADER
@@ -24,7 +24,8 @@ namespace llvm {
 class NVPTXRegisterInfo : public NVPTXGenRegisterInfo {
 private:
   // Hold Strings that can be free'd all together with NVPTXRegisterInfo
-  ManagedStringPool ManagedStrPool;
+  BumpPtrAllocator StrAlloc;
+  UniqueStringSaver StrPool;
 
 public:
   NVPTXRegisterInfo();
@@ -45,14 +46,14 @@ class NVPTXRegisterInfo : public NVPTXGenRegisterInfo {
   Register getFrameRegister(const MachineFunction &MF) const override;
   Register getFrameLocalRegister(const MachineFunction &MF) const;
 
-  ManagedStringPool *getStrPool() const {
-    return const_cast<ManagedStringPool *>(&ManagedStrPool);
+  UniqueStringSaver &getStrPool() const {
+    return const_cast<UniqueStringSaver &>(StrPool);
   }
 
   const char *getName(unsigned RegNo) const {
     std::stringstream O;
     O << "reg" << RegNo;
-    return getStrPool()->getManagedString(O.str().c_str())->c_str();
+    return getStrPool().save(O.str()).data();
   }
 
 };

diff  --git a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
index 4ef989cb78fc7..3c379bb9433df 100644
--- a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
@@ -124,7 +124,8 @@ NVPTXTargetMachine::NVPTXTargetMachine(const Target &T, const Triple &TT,
                         getEffectiveCodeModel(CM, CodeModel::Small), OL),
       is64bit(is64bit), UseShortPointers(UseShortPointersOpt),
       TLOF(std::make_unique<NVPTXTargetObjectFile>()),
-      Subtarget(TT, std::string(CPU), std::string(FS), *this) {
+      Subtarget(TT, std::string(CPU), std::string(FS), *this),
+      StrPool(StrAlloc) {
   if (TT.getOS() == Triple::NVCL)
     drvInterface = NVPTX::NVCL;
   else

diff  --git a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
index 12005e0ebf39f..843c3a218e1dd 100644
--- a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
+++ b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
@@ -13,7 +13,6 @@
 #ifndef LLVM_LIB_TARGET_NVPTX_NVPTXTARGETMACHINE_H
 #define LLVM_LIB_TARGET_NVPTX_NVPTXTARGETMACHINE_H
 
-#include "ManagedStringPool.h"
 #include "NVPTXSubtarget.h"
 #include "llvm/Target/TargetMachine.h"
 #include <optional>
@@ -32,7 +31,8 @@ class NVPTXTargetMachine : public LLVMTargetMachine {
   NVPTXSubtarget Subtarget;
 
   // Hold Strings that can be free'd all together with NVPTXTargetMachine
-  ManagedStringPool ManagedStrPool;
+  BumpPtrAllocator StrAlloc;
+  UniqueStringSaver StrPool;
 
 public:
   NVPTXTargetMachine(const Target &T, const Triple &TT, StringRef CPU,
@@ -48,8 +48,8 @@ class NVPTXTargetMachine : public LLVMTargetMachine {
   bool is64Bit() const { return is64bit; }
   bool useShortPointers() const { return UseShortPointers; }
   NVPTX::DrvInterface getDrvInterface() const { return drvInterface; }
-  ManagedStringPool *getManagedStrPool() const {
-    return const_cast<ManagedStringPool *>(&ManagedStrPool);
+  UniqueStringSaver &getStrPool() const {
+    return const_cast<UniqueStringSaver &>(StrPool);
   }
 
   TargetPassConfig *createPassConfig(PassManagerBase &PM) override;


        


More information about the llvm-commits mailing list