[llvm] Stop abusing Twine in DiagnosticInfo (PR #136371)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 18 14:29:42 PDT 2025


https://github.com/bogner created https://github.com/llvm/llvm-project/pull/136371

This switches the various DiagnosticInfo subclasses that store an un-owned string to use StringRef for this instead of Twine.

Storing a `Twine &` like most of these were doing makes it very easy to introduce memory corruption bugs:

```c++
void f(std::string &Msg) {
  auto Diag = DiagnosticInfoGeneric(Msg);
  Ctx.diagnose(Diag); // Bug! We're referring to the temporary Twine!
  Ctx.diagnose(DiagnosticInfoGeneric(Msg); // this works, I guess...
};
```

This was sort of mitigated in DiagnosticInfoUnsupported, which actually copies the Twine, but copying Twines is always a questionable practice.

Instead, we store a StringRef, which makes it much more explicit that ownership of the storage is the caller's problem.

>From 7327ce46e221207e0ffb66ec7b2dc16aed4ead3c Mon Sep 17 00:00:00 2001
From: Justin Bogner <mail at justinbogner.com>
Date: Fri, 18 Apr 2025 14:20:28 -0700
Subject: [PATCH] Stop abusing Twine in DiagnosticInfo

This switches the various DiagnosticInfo subclasses that store an un-owned
string to use StringRef for this instead of Twine.

Storing a `Twine &` like most of these were doing makes it very easy to
introduce memory corruption bugs:

```c++
void f(std::string &Msg) {
  auto Diag = DiagnosticInfoGeneric(Msg);
  Ctx.diagnose(Diag); // Bug! We're referring to the temporary Twine!
  Ctx.diagnose(DiagnosticInfoGeneric(Msg); // this works, I guess...
};
```

This was sort of mitigated in DiagnosticInfoUnsupported, which actually copies
the Twine, but copying Twines is always a questionable practice.

Instead, we store a StringRef, which makes it much more explicit that ownership
of the storage is the caller's problem.
---
 llvm/include/llvm/IR/DiagnosticInfo.h         | 81 ++++++++---------
 .../llvm/ProfileData/SampleProfReader.h       |  5 +-
 .../Utils/SampleProfileLoaderBaseImpl.h       | 33 ++++---
 .../AsmPrinter/AsmPrinterInlineAsm.cpp        |  7 +-
 llvm/lib/CodeGen/MachineInstr.cpp             |  8 +-
 .../SelectionDAG/SelectionDAGBuilder.cpp      | 10 +-
 llvm/lib/IR/DiagnosticInfo.cpp                | 11 +--
 llvm/lib/IR/LLVMContext.cpp                   |  7 +-
 llvm/lib/Target/DirectX/DXILRootSignature.cpp |  5 +-
 .../Instrumentation/PGOInstrumentation.cpp    | 91 +++++++++----------
 llvm/lib/Transforms/Utils/MisExpect.cpp       |  9 +-
 11 files changed, 137 insertions(+), 130 deletions(-)

diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h
index 8743f8058c382..151f323447bfb 100644
--- a/llvm/include/llvm/IR/DiagnosticInfo.h
+++ b/llvm/include/llvm/IR/DiagnosticInfo.h
@@ -18,7 +18,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Twine.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -139,22 +138,22 @@ class DiagnosticInfo {
 using DiagnosticHandlerFunction = std::function<void(const DiagnosticInfo &)>;
 
 class DiagnosticInfoGeneric : public DiagnosticInfo {
-  const Twine &MsgStr;
+  StringRef MsgStr;
   const Instruction *Inst = nullptr;
 
 public:
   /// \p MsgStr is the message to be reported to the frontend.
   /// This class does not copy \p MsgStr, therefore the reference must be valid
   /// for the whole life time of the Diagnostic.
-  DiagnosticInfoGeneric(const Twine &MsgStr,
+  DiagnosticInfoGeneric(StringRef MsgStr,
                         DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_Generic, Severity), MsgStr(MsgStr) {}
 
-  DiagnosticInfoGeneric(const Instruction *I, const Twine &ErrMsg,
+  DiagnosticInfoGeneric(const Instruction *I, StringRef ErrMsg,
                         DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_Generic, Severity), MsgStr(ErrMsg), Inst(I) {}
 
-  const Twine &getMsgStr() const { return MsgStr; }
+  StringRef getMsgStr() const { return MsgStr; }
   const Instruction *getInstruction() const { return Inst; }
 
   /// \see DiagnosticInfo::print.
@@ -172,7 +171,7 @@ class DiagnosticInfoInlineAsm : public DiagnosticInfo {
   /// Optional line information. 0 if not set.
   uint64_t LocCookie = 0;
   /// Message to be reported.
-  const Twine &MsgStr;
+  StringRef MsgStr;
   /// Optional origin of the problem.
   const Instruction *Instr = nullptr;
 
@@ -181,7 +180,7 @@ class DiagnosticInfoInlineAsm : public DiagnosticInfo {
   /// \p MsgStr gives the message.
   /// This class does not copy \p MsgStr, therefore the reference must be valid
   /// for the whole life time of the Diagnostic.
-  DiagnosticInfoInlineAsm(uint64_t LocCookie, const Twine &MsgStr,
+  DiagnosticInfoInlineAsm(uint64_t LocCookie, StringRef MsgStr,
                           DiagnosticSeverity Severity = DS_Error);
 
   /// \p Instr gives the original instruction that triggered the diagnostic.
@@ -189,11 +188,11 @@ class DiagnosticInfoInlineAsm : public DiagnosticInfo {
   /// This class does not copy \p MsgStr, therefore the reference must be valid
   /// for the whole life time of the Diagnostic.
   /// Same for \p I.
-  DiagnosticInfoInlineAsm(const Instruction &I, const Twine &MsgStr,
+  DiagnosticInfoInlineAsm(const Instruction &I, StringRef MsgStr,
                           DiagnosticSeverity Severity = DS_Error);
 
   uint64_t getLocCookie() const { return LocCookie; }
-  const Twine &getMsgStr() const { return MsgStr; }
+  StringRef getMsgStr() const { return MsgStr; }
   const Instruction *getInstruction() const { return Instr; }
 
   /// \see DiagnosticInfo::print.
@@ -258,15 +257,15 @@ class DiagnosticInfoIgnoringInvalidDebugMetadata : public DiagnosticInfo {
 class DiagnosticInfoSampleProfile : public DiagnosticInfo {
 public:
   DiagnosticInfoSampleProfile(StringRef FileName, unsigned LineNum,
-                              const Twine &Msg,
+                              StringRef Msg,
                               DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_SampleProfile, Severity), FileName(FileName),
         LineNum(LineNum), Msg(Msg) {}
-  DiagnosticInfoSampleProfile(StringRef FileName, const Twine &Msg,
+  DiagnosticInfoSampleProfile(StringRef FileName, StringRef Msg,
                               DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_SampleProfile, Severity), FileName(FileName),
         Msg(Msg) {}
-  DiagnosticInfoSampleProfile(const Twine &Msg,
+  DiagnosticInfoSampleProfile(StringRef Msg,
                               DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_SampleProfile, Severity), Msg(Msg) {}
 
@@ -279,7 +278,7 @@ class DiagnosticInfoSampleProfile : public DiagnosticInfo {
 
   StringRef getFileName() const { return FileName; }
   unsigned getLineNum() const { return LineNum; }
-  const Twine &getMsg() const { return Msg; }
+  StringRef getMsg() const { return Msg; }
 
 private:
   /// Name of the input file associated with this diagnostic.
@@ -290,13 +289,13 @@ class DiagnosticInfoSampleProfile : public DiagnosticInfo {
   unsigned LineNum = 0;
 
   /// Message to report.
-  const Twine &Msg;
+  StringRef Msg;
 };
 
 /// Diagnostic information for the PGO profiler.
 class DiagnosticInfoPGOProfile : public DiagnosticInfo {
 public:
-  DiagnosticInfoPGOProfile(const char *FileName, const Twine &Msg,
+  DiagnosticInfoPGOProfile(const char *FileName, StringRef Msg,
                            DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfo(DK_PGOProfile, Severity), FileName(FileName), Msg(Msg) {}
 
@@ -308,14 +307,14 @@ class DiagnosticInfoPGOProfile : public DiagnosticInfo {
   }
 
   const char *getFileName() const { return FileName; }
-  const Twine &getMsg() const { return Msg; }
+  StringRef getMsg() const { return Msg; }
 
 private:
   /// Name of the input file associated with this diagnostic.
   const char *FileName;
 
   /// Message to report.
-  const Twine &Msg;
+  StringRef Msg;
 };
 
 class DiagnosticLocation {
@@ -364,7 +363,7 @@ class DiagnosticInfoWithLocationBase : public DiagnosticInfo {
 
   /// Return the absolute path tot the file.
   std::string getAbsolutePath() const;
-  
+
   const Function &getFunction() const { return Fn; }
   DiagnosticLocation getLocation() const { return Loc; }
 
@@ -379,19 +378,19 @@ class DiagnosticInfoWithLocationBase : public DiagnosticInfo {
 class DiagnosticInfoGenericWithLoc : public DiagnosticInfoWithLocationBase {
 private:
   /// Message to be reported.
-  const Twine &MsgStr;
+  StringRef MsgStr;
 
 public:
   /// \p MsgStr is the message to be reported to the frontend.
   /// This class does not copy \p MsgStr, therefore the reference must be valid
   /// for the whole life time of the Diagnostic.
-  DiagnosticInfoGenericWithLoc(const Twine &MsgStr, const Function &Fn,
+  DiagnosticInfoGenericWithLoc(StringRef MsgStr, const Function &Fn,
                                const DiagnosticLocation &Loc,
                                DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfoWithLocationBase(DK_GenericWithLoc, Severity, Fn, Loc),
         MsgStr(MsgStr) {}
 
-  const Twine &getMsgStr() const { return MsgStr; }
+  StringRef getMsgStr() const { return MsgStr; }
 
   /// \see DiagnosticInfo::print.
   void print(DiagnosticPrinter &DP) const override;
@@ -404,20 +403,20 @@ class DiagnosticInfoGenericWithLoc : public DiagnosticInfoWithLocationBase {
 class DiagnosticInfoRegAllocFailure : public DiagnosticInfoWithLocationBase {
 private:
   /// Message to be reported.
-  const Twine &MsgStr;
+  StringRef MsgStr;
 
 public:
   /// \p MsgStr is the message to be reported to the frontend.
   /// This class does not copy \p MsgStr, therefore the reference must be valid
   /// for the whole life time of the Diagnostic.
-  DiagnosticInfoRegAllocFailure(const Twine &MsgStr, const Function &Fn,
+  DiagnosticInfoRegAllocFailure(StringRef MsgStr, const Function &Fn,
                                 const DiagnosticLocation &DL,
                                 DiagnosticSeverity Severity = DS_Error);
 
-  DiagnosticInfoRegAllocFailure(const Twine &MsgStr, const Function &Fn,
+  DiagnosticInfoRegAllocFailure(StringRef MsgStr, const Function &Fn,
                                 DiagnosticSeverity Severity = DS_Error);
 
-  const Twine &getMsgStr() const { return MsgStr; }
+  StringRef getMsgStr() const { return MsgStr; }
 
   /// \see DiagnosticInfo::print.
   void print(DiagnosticPrinter &DP) const override;
@@ -707,7 +706,7 @@ class DiagnosticInfoIROptimization : public DiagnosticInfoOptimizationBase {
   DiagnosticInfoIROptimization(enum DiagnosticKind Kind,
                                enum DiagnosticSeverity Severity,
                                const char *PassName, const Function &Fn,
-                               const DiagnosticLocation &Loc, const Twine &Msg)
+                               const DiagnosticLocation &Loc, StringRef Msg)
       : DiagnosticInfoOptimizationBase(Kind, Severity, PassName, "", Fn, Loc) {
     *this << Msg.str();
   }
@@ -764,7 +763,7 @@ class OptimizationRemark : public DiagnosticInfoIROptimization {
   /// Note that this class does not copy this message, so this reference
   /// must be valid for the whole life time of the diagnostic.
   OptimizationRemark(const char *PassName, const Function &Fn,
-                     const DiagnosticLocation &Loc, const Twine &Msg)
+                     const DiagnosticLocation &Loc, StringRef Msg)
       : DiagnosticInfoIROptimization(DK_OptimizationRemark, DS_Remark, PassName,
                                      Fn, Loc, Msg) {}
 };
@@ -810,7 +809,7 @@ class OptimizationRemarkMissed : public DiagnosticInfoIROptimization {
   /// Note that this class does not copy this message, so this reference
   /// must be valid for the whole life time of the diagnostic.
   OptimizationRemarkMissed(const char *PassName, const Function &Fn,
-                           const DiagnosticLocation &Loc, const Twine &Msg)
+                           const DiagnosticLocation &Loc, StringRef Msg)
       : DiagnosticInfoIROptimization(DK_OptimizationRemarkMissed, DS_Remark,
                                      PassName, Fn, Loc, Msg) {}
 };
@@ -863,7 +862,7 @@ class OptimizationRemarkAnalysis : public DiagnosticInfoIROptimization {
 protected:
   OptimizationRemarkAnalysis(enum DiagnosticKind Kind, const char *PassName,
                              const Function &Fn, const DiagnosticLocation &Loc,
-                             const Twine &Msg)
+                             StringRef Msg)
       : DiagnosticInfoIROptimization(Kind, DS_Remark, PassName, Fn, Loc, Msg) {}
 
   OptimizationRemarkAnalysis(enum DiagnosticKind Kind, const char *PassName,
@@ -882,7 +881,7 @@ class OptimizationRemarkAnalysis : public DiagnosticInfoIROptimization {
   /// this class does not copy this message, so this reference must be valid for
   /// the whole life time of the diagnostic.
   OptimizationRemarkAnalysis(const char *PassName, const Function &Fn,
-                             const DiagnosticLocation &Loc, const Twine &Msg)
+                             const DiagnosticLocation &Loc, StringRef Msg)
       : DiagnosticInfoIROptimization(DK_OptimizationRemarkAnalysis, DS_Remark,
                                      PassName, Fn, Loc, Msg) {}
 };
@@ -924,7 +923,7 @@ class OptimizationRemarkAnalysisFPCommute : public OptimizationRemarkAnalysis {
   /// diagnostic.
   OptimizationRemarkAnalysisFPCommute(const char *PassName, const Function &Fn,
                                       const DiagnosticLocation &Loc,
-                                      const Twine &Msg)
+                                      StringRef Msg)
       : OptimizationRemarkAnalysis(DK_OptimizationRemarkAnalysisFPCommute,
                                    PassName, Fn, Loc, Msg) {}
 };
@@ -965,7 +964,7 @@ class OptimizationRemarkAnalysisAliasing : public OptimizationRemarkAnalysis {
   /// diagnostic.
   OptimizationRemarkAnalysisAliasing(const char *PassName, const Function &Fn,
                                      const DiagnosticLocation &Loc,
-                                     const Twine &Msg)
+                                     StringRef Msg)
       : OptimizationRemarkAnalysis(DK_OptimizationRemarkAnalysisAliasing,
                                    PassName, Fn, Loc, Msg) {}
 };
@@ -991,10 +990,10 @@ class DiagnosticInfoMIRParser : public DiagnosticInfo {
 
 /// Diagnostic information for IR instrumentation reporting.
 class DiagnosticInfoInstrumentation : public DiagnosticInfo {
-  const Twine &Msg;
+  StringRef Msg;
 
 public:
-  DiagnosticInfoInstrumentation(const Twine &DiagMsg,
+  DiagnosticInfoInstrumentation(StringRef DiagMsg,
                                 DiagnosticSeverity Severity = DS_Warning)
       : DiagnosticInfo(DK_Instrumentation, Severity), Msg(DiagMsg) {}
 
@@ -1038,7 +1037,7 @@ class DiagnosticInfoOptimizationFailure : public DiagnosticInfoIROptimization {
   /// of the diagnostic.
   DiagnosticInfoOptimizationFailure(const Function &Fn,
                                     const DiagnosticLocation &Loc,
-                                    const Twine &Msg)
+                                    StringRef Msg)
       : DiagnosticInfoIROptimization(DK_OptimizationFailure, DS_Warning,
                                      nullptr, Fn, Loc, Msg) {}
 
@@ -1062,7 +1061,7 @@ class DiagnosticInfoOptimizationFailure : public DiagnosticInfoIROptimization {
 /// Diagnostic information for unsupported feature in backend.
 class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
 private:
-  Twine Msg;
+  StringRef Msg;
 
 public:
   /// \p Fn is the function where the diagnostic is being emitted. \p Loc is
@@ -1072,7 +1071,7 @@ class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
   /// copy this message, so this reference must be valid for the whole life time
   /// of the diagnostic.
   DiagnosticInfoUnsupported(
-      const Function &Fn, const Twine &Msg,
+      const Function &Fn, StringRef Msg,
       const DiagnosticLocation &Loc = DiagnosticLocation(),
       DiagnosticSeverity Severity = DS_Error)
       : DiagnosticInfoWithLocationBase(DK_Unsupported, Severity, Fn, Loc),
@@ -1082,7 +1081,7 @@ class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
     return DI->getKind() == DK_Unsupported;
   }
 
-  const Twine &getMessage() const { return Msg; }
+  const StringRef getMessage() const { return Msg; }
 
   void print(DiagnosticPrinter &DP) const override;
 };
@@ -1090,7 +1089,7 @@ class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
 /// Diagnostic information for MisExpect analysis.
 class DiagnosticInfoMisExpect : public DiagnosticInfoWithLocationBase {
 public:
-  DiagnosticInfoMisExpect(const Instruction *Inst, Twine &Msg);
+  DiagnosticInfoMisExpect(const Instruction *Inst, StringRef Msg);
 
   /// \see DiagnosticInfo::print.
   void print(DiagnosticPrinter &DP) const override;
@@ -1099,11 +1098,11 @@ class DiagnosticInfoMisExpect : public DiagnosticInfoWithLocationBase {
     return DI->getKind() == DK_MisExpect;
   }
 
-  const Twine &getMsg() const { return Msg; }
+  StringRef getMsg() const { return Msg; }
 
 private:
   /// Message to report.
-  const Twine &Msg;
+  StringRef Msg;
 };
 
 static DiagnosticSeverity getDiagnosticSeverity(SourceMgr::DiagKind DK) {
diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h
index 76c7cecded629..d3f29d52f0c4b 100644
--- a/llvm/include/llvm/ProfileData/SampleProfReader.h
+++ b/llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -449,8 +449,9 @@ class SampleProfileReader {
 
   /// Report a parse error message.
   void reportError(int64_t LineNumber, const Twine &Msg) const {
-    Ctx.diagnose(DiagnosticInfoSampleProfile(Buffer->getBufferIdentifier(),
-                                             LineNumber, Msg));
+    SmallString<128> Storage;
+    Ctx.diagnose(DiagnosticInfoSampleProfile(
+        Buffer->getBufferIdentifier(), LineNumber, Msg.toStringRef(Storage)));
   }
 
   /// Create a sample profile reader appropriate to the file format.
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
index d714dba4a9687..1a63d3ed96438 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/Analysis/LoopInfo.h"
@@ -39,6 +40,7 @@
 #include "llvm/ProfileData/SampleProf.h"
 #include "llvm/ProfileData/SampleProfReader.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/GenericDomTree.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/SampleProfileInference.h"
@@ -1104,12 +1106,13 @@ void SampleProfileLoaderBaseImpl<BT>::emitCoverageRemarks(FunctionT &F) {
     unsigned Used = CoverageTracker.countUsedRecords(Samples, PSI);
     unsigned Total = CoverageTracker.countBodyRecords(Samples, PSI);
     unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
+    SmallString<128> Msg =
+        formatv("{0} of {1} available profile records ({2}%) were applied",
+                Used, Total, Coverage);
     if (Coverage < SampleProfileRecordCoverage) {
-      Func.getContext().diagnose(DiagnosticInfoSampleProfile(
-          Func.getSubprogram()->getFilename(), getFunctionLoc(F),
-          Twine(Used) + " of " + Twine(Total) + " available profile records (" +
-              Twine(Coverage) + "%) were applied",
-          DS_Warning));
+      Func.getContext().diagnose(
+          DiagnosticInfoSampleProfile(Func.getSubprogram()->getFilename(),
+                                      getFunctionLoc(F), Msg, DS_Warning));
     }
   }
 
@@ -1117,12 +1120,13 @@ void SampleProfileLoaderBaseImpl<BT>::emitCoverageRemarks(FunctionT &F) {
     uint64_t Used = CoverageTracker.getTotalUsedSamples();
     uint64_t Total = CoverageTracker.countBodySamples(Samples, PSI);
     unsigned Coverage = CoverageTracker.computeCoverage(Used, Total);
+    SmallString<128> Msg =
+        formatv("{0} of {1} available profile samples ({2}%) were applied",
+                Used, Total, Coverage);
     if (Coverage < SampleProfileSampleCoverage) {
-      Func.getContext().diagnose(DiagnosticInfoSampleProfile(
-          Func.getSubprogram()->getFilename(), getFunctionLoc(F),
-          Twine(Used) + " of " + Twine(Total) + " available profile samples (" +
-              Twine(Coverage) + "%) were applied",
-          DS_Warning));
+      Func.getContext().diagnose(
+          DiagnosticInfoSampleProfile(Func.getSubprogram()->getFilename(),
+                                      getFunctionLoc(F), Msg, DS_Warning));
     }
   }
 }
@@ -1149,10 +1153,11 @@ unsigned SampleProfileLoaderBaseImpl<BT>::getFunctionLoc(FunctionT &F) {
 
   // If the start of \p F is missing, emit a diagnostic to inform the user
   // about the missed opportunity.
-  Func.getContext().diagnose(DiagnosticInfoSampleProfile(
-      "No debug information found in function " + Func.getName() +
-          ": Function profile not used",
-      DS_Warning));
+  SmallString<128> Msg = formatv(
+      "No debug information found in function {0}: Function profile not used",
+      Func.getName());
+
+  Func.getContext().diagnose(DiagnosticInfoSampleProfile(Msg, DS_Warning));
   return 0;
 }
 
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
index 59fc4cfc23e10..217ce0a298e19 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
@@ -34,6 +34,7 @@
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
@@ -313,9 +314,9 @@ static void EmitInlineAsmStr(const char *AsmStr, const MachineInstr *MI,
         }
         if (Error) {
           const Function &Fn = MI->getMF()->getFunction();
-          Fn.getContext().diagnose(DiagnosticInfoInlineAsm(
-              LocCookie,
-              "invalid operand in inline asm: '" + Twine(AsmStr) + "'"));
+          SmallString<128> Msg =
+              formatv("invalid operand in inline asm: '{0}'", AsmStr);
+          Fn.getContext().diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg));
         }
       }
       break;
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 33910d0ec6aeb..70e0314f3d281 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -2332,13 +2332,15 @@ void MachineInstr::emitInlineAsmError(const Twine &Msg) const {
           ? mdconst::extract<ConstantInt>(LocMD->getOperand(0))->getZExtValue()
           : 0;
   LLVMContext &Ctx = getMF()->getFunction().getContext();
-  Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg));
+  SmallString<128> Storage;
+  Ctx.diagnose(DiagnosticInfoInlineAsm(LocCookie, Msg.toStringRef(Storage)));
 }
 
 void MachineInstr::emitGenericError(const Twine &Msg) const {
   const Function &Fn = getMF()->getFunction();
-  Fn.getContext().diagnose(
-      DiagnosticInfoGenericWithLoc(Msg, Fn, getDebugLoc()));
+  SmallString<128> Storage;
+  Fn.getContext().diagnose(DiagnosticInfoGenericWithLoc(
+      Msg.toStringRef(Storage), Fn, getDebugLoc()));
 }
 
 MachineInstrBuilder llvm::BuildMI(MachineFunction &MF, const DebugLoc &DL,
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index d7a67cca3c197..ef253d938739e 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -316,15 +316,16 @@ getCopyFromParts(SelectionDAG &DAG, const SDLoc &DL, const SDValue *Parts,
 }
 
 static void diagnosePossiblyInvalidConstraint(LLVMContext &Ctx, const Value *V,
-                                              const Twine &ErrMsg) {
+                                              StringRef ErrMsg) {
   const Instruction *I = dyn_cast_or_null<Instruction>(V);
   if (!I)
     return Ctx.emitError(ErrMsg);
 
   if (const CallInst *CI = dyn_cast<CallInst>(I))
     if (CI->isInlineAsm()) {
-      return Ctx.diagnose(DiagnosticInfoInlineAsm(
-          *CI, ErrMsg + ", possible invalid constraint for vector type"));
+      SmallString<128> Msg(
+          {ErrMsg, ", possible invalid constraint for vector type"});
+      return Ctx.diagnose(DiagnosticInfoInlineAsm(*CI, Msg));
     }
 
   return Ctx.emitError(I, ErrMsg);
@@ -10445,7 +10446,8 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
 void SelectionDAGBuilder::emitInlineAsmError(const CallBase &Call,
                                              const Twine &Message) {
   LLVMContext &Ctx = *DAG.getContext();
-  Ctx.diagnose(DiagnosticInfoInlineAsm(Call, Message));
+  SmallString<128> Storage;
+  Ctx.diagnose(DiagnosticInfoInlineAsm(Call, Message.toStringRef(Storage)));
 
   // Make sure we leave the DAG in a valid state
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp
index 0e526ada4b405..938d8642ff14c 100644
--- a/llvm/lib/IR/DiagnosticInfo.cpp
+++ b/llvm/lib/IR/DiagnosticInfo.cpp
@@ -13,7 +13,6 @@
 
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/BasicBlock.h"
@@ -57,13 +56,13 @@ void DiagnosticInfoGenericWithLoc::print(DiagnosticPrinter &DP) const {
 }
 
 DiagnosticInfoInlineAsm::DiagnosticInfoInlineAsm(uint64_t LocCookie,
-                                                 const Twine &MsgStr,
+                                                 StringRef MsgStr,
                                                  DiagnosticSeverity Severity)
     : DiagnosticInfo(DK_InlineAsm, Severity), LocCookie(LocCookie),
       MsgStr(MsgStr) {}
 
 DiagnosticInfoInlineAsm::DiagnosticInfoInlineAsm(const Instruction &I,
-                                                 const Twine &MsgStr,
+                                                 StringRef MsgStr,
                                                  DiagnosticSeverity Severity)
     : DiagnosticInfo(DK_InlineAsm, Severity), MsgStr(MsgStr), Instr(&I) {
   if (const MDNode *SrcLoc = I.getMetadata("srcloc")) {
@@ -81,14 +80,14 @@ void DiagnosticInfoInlineAsm::print(DiagnosticPrinter &DP) const {
 }
 
 DiagnosticInfoRegAllocFailure::DiagnosticInfoRegAllocFailure(
-    const Twine &MsgStr, const Function &Fn, const DiagnosticLocation &DL,
+    StringRef MsgStr, const Function &Fn, const DiagnosticLocation &DL,
     DiagnosticSeverity Severity)
     : DiagnosticInfoWithLocationBase(DK_RegAllocFailure, Severity, Fn,
                                      DL.isValid() ? DL : Fn.getSubprogram()),
       MsgStr(MsgStr) {}
 
 DiagnosticInfoRegAllocFailure::DiagnosticInfoRegAllocFailure(
-    const Twine &MsgStr, const Function &Fn, DiagnosticSeverity Severity)
+    StringRef MsgStr, const Function &Fn, DiagnosticSeverity Severity)
     : DiagnosticInfoWithLocationBase(DK_RegAllocFailure, Severity, Fn,
                                      Fn.getSubprogram()),
       MsgStr(MsgStr) {}
@@ -443,7 +442,7 @@ std::string DiagnosticInfoOptimizationBase::getMsg() const {
 }
 
 DiagnosticInfoMisExpect::DiagnosticInfoMisExpect(const Instruction *Inst,
-                                                 Twine &Msg)
+                                                 StringRef Msg)
     : DiagnosticInfoWithLocationBase(DK_MisExpect, DS_Warning,
                                      *Inst->getParent()->getParent(),
                                      Inst->getDebugLoc()),
diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp
index 447e5d92e0b99..1000d22435337 100644
--- a/llvm/lib/IR/LLVMContext.cpp
+++ b/llvm/lib/IR/LLVMContext.cpp
@@ -13,6 +13,7 @@
 
 #include "llvm/IR/LLVMContext.h"
 #include "LLVMContextImpl.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -204,12 +205,14 @@ void LLVMContext::yield() {
 }
 
 void LLVMContext::emitError(const Twine &ErrorStr) {
-  diagnose(DiagnosticInfoGeneric(ErrorStr));
+  SmallString<128> Storage;
+  diagnose(DiagnosticInfoGeneric(ErrorStr.toStringRef(Storage)));
 }
 
 void LLVMContext::emitError(const Instruction *I, const Twine &ErrorStr) {
   assert(I && "Invalid instruction");
-  diagnose(DiagnosticInfoGeneric(I, ErrorStr));
+  SmallString<128> Storage;
+  diagnose(DiagnosticInfoGeneric(I, ErrorStr.toStringRef(Storage)));
 }
 
 static bool isDiagnosticEnabled(const DiagnosticInfo &DI) {
diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index 3ba0535e0114b..ab701da324355 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -34,9 +34,8 @@
 using namespace llvm;
 using namespace llvm::dxil;
 
-static bool reportError(LLVMContext *Ctx, Twine Message,
-                        DiagnosticSeverity Severity = DS_Error) {
-  Ctx->diagnose(DiagnosticInfoGeneric(Message, Severity));
+static bool reportError(LLVMContext *Ctx, Twine Message) {
+  Ctx->emitError(Message);
   return true;
 }
 
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 7c73c16db02c8..a319635aa18e9 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -404,6 +404,13 @@ class FunctionInstrumenter final {
 };
 } // namespace
 
+static void emitDiagnostic(LLVMContext &Ctx, StringRef Filename,
+                           const Twine &Msg, DiagnosticSeverity Severity) {
+  SmallString<128> Storage;
+  Ctx.diagnose(DiagnosticInfoPGOProfile(Filename.data(),
+                                        Msg.toStringRef(Storage), Severity));
+}
+
 // Return a string describing the branch condition that can be
 // used in static branch probability heuristics:
 static std::string getBranchCondString(Instruction *TI) {
@@ -1434,8 +1441,7 @@ void PGOUseFunc::handleInstrProfError(Error Err, uint64_t MismatchedFuncSum) {
         std::string(" up to ") + std::to_string(MismatchedFuncSum) +
         std::string(" count discarded");
 
-    Ctx.diagnose(
-        DiagnosticInfoPGOProfile(M->getName().data(), Msg, DS_Warning));
+    emitDiagnostic(Ctx, M->getName(), Msg, DS_Warning);
   });
 }
 
@@ -1478,12 +1484,12 @@ bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros,
   if (!setInstrumentedCounts(CountFromProfile)) {
     LLVM_DEBUG(
         dbgs() << "Inconsistent number of counts, skipping this function");
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M->getName().data(),
+    emitDiagnostic(
+        Ctx, M->getName(),
         Twine("Inconsistent number of counts in ") + F.getName().str() +
             Twine(": the profile may be stale or there is a function name "
                   "collision."),
-        DS_Warning));
+        DS_Warning);
     return false;
   }
   ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
@@ -1590,12 +1596,11 @@ void PGOUseFunc::populateCoverage(IndexedInstrProfReader *PGOReader) {
       ++NumCoveredBlocks;
   }
   if (PGOVerifyBFI && NumCorruptCoverage) {
-    auto &Ctx = M->getContext();
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M->getName().data(),
-        Twine("Found inconsistent block coverage for function ") + F.getName() +
-            " in " + Twine(NumCorruptCoverage) + " blocks.",
-        DS_Warning));
+    emitDiagnostic(M->getContext(), M->getName(),
+                   Twine("Found inconsistent block coverage for function ") +
+                       F.getName() + " in " + Twine(NumCorruptCoverage) +
+                       " blocks.",
+                   DS_Warning);
   }
   if (PGOViewBlockCoverageGraph)
     FuncInfo.BCI->viewBlockCoverageGraph(&Coverage);
@@ -1729,13 +1734,11 @@ void PGOUseFunc::setBranchWeights() {
       // A zero MaxCount can come about when we have a BB with a positive
       // count, and whose successor blocks all have 0 count. This can happen
       // when there is no exit block and the code exits via a noreturn function.
-      auto &Ctx = M->getContext();
-      Ctx.diagnose(DiagnosticInfoPGOProfile(
-          M->getName().data(),
-          Twine("Profile in ") + F.getName().str() +
-              Twine(" partially ignored") +
-              Twine(", possibly due to the lack of a return path."),
-          DS_Warning));
+      emitDiagnostic(M->getContext(), M->getName(),
+                     Twine("Profile in ") + F.getName().str() +
+                         Twine(" partially ignored") +
+                         Twine(", possibly due to the lack of a return path."),
+                     DS_Warning);
     }
   }
 }
@@ -1865,14 +1868,12 @@ void PGOUseFunc::annotateValueSites(uint32_t Kind) {
     FuncInfo.ValueSites[IPVK_VTableTarget] = VPC.get(IPVK_VTableTarget);
   auto &ValueSites = FuncInfo.ValueSites[Kind];
   if (NumValueSites != ValueSites.size()) {
-    auto &Ctx = M->getContext();
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M->getName().data(),
-        Twine("Inconsistent number of value sites for ") +
-            Twine(ValueProfKindDescr[Kind]) + Twine(" profiling in \"") +
-            F.getName().str() +
-            Twine("\", possibly due to the use of a stale profile."),
-        DS_Warning));
+    emitDiagnostic(M->getContext(), M->getName(),
+                   Twine("Inconsistent number of value sites for ") +
+                       Twine(ValueProfKindDescr[Kind]) +
+                       Twine(" profiling in \"") + F.getName().str() +
+                       Twine("\", possibly due to the use of a stale profile."),
+                   DS_Warning);
     return;
   }
 
@@ -1962,13 +1963,11 @@ static bool InstrumentAllFunctions(
     createIRLevelProfileFlagVar(M, InstrumentationType);
 
   Triple TT(M.getTargetTriple());
-  LLVMContext &Ctx = M.getContext();
   if (!TT.isOSBinFormatELF() && EnableVTableValueProfiling)
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        M.getName().data(),
-        Twine("VTable value profiling is presently not "
-              "supported for non-ELF object formats"),
-        DS_Warning));
+    emitDiagnostic(M.getContext(), M.getName(),
+                   Twine("VTable value profiling is presently not "
+                         "supported for non-ELF object formats"),
+                   DS_Warning);
   std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
   collectComdatMembers(M, ComdatMembers);
 
@@ -2161,8 +2160,7 @@ static bool annotateAllFunctions(
                                                     ProfileRemappingFileName);
   if (Error E = ReaderOrErr.takeError()) {
     handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
-      Ctx.diagnose(
-          DiagnosticInfoPGOProfile(ProfileFileName.data(), EI.message()));
+      emitDiagnostic(Ctx, ProfileFileName, EI.message(), DS_Error);
     });
     return false;
   }
@@ -2170,8 +2168,7 @@ static bool annotateAllFunctions(
   std::unique_ptr<IndexedInstrProfReader> PGOReader =
       std::move(ReaderOrErr.get());
   if (!PGOReader) {
-    Ctx.diagnose(DiagnosticInfoPGOProfile(ProfileFileName.data(),
-                                          StringRef("Cannot get PGOReader")));
+    emitDiagnostic(Ctx, ProfileFileName, "Cannot get PGOReader", DS_Error);
     return false;
   }
   if (!PGOReader->hasCSIRLevelProfile() && IsCS)
@@ -2179,14 +2176,15 @@ static bool annotateAllFunctions(
 
   // TODO: might need to change the warning once the clang option is finalized.
   if (!PGOReader->isIRLevelProfile()) {
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        ProfileFileName.data(), "Not an IR level instrumentation profile"));
+    emitDiagnostic(Ctx, ProfileFileName,
+                   "Not an IR level instrumentation profile", DS_Error);
     return false;
   }
   if (PGOReader->functionEntryOnly()) {
-    Ctx.diagnose(DiagnosticInfoPGOProfile(
-        ProfileFileName.data(),
-        "Function entry profiles are not yet supported for optimization"));
+    emitDiagnostic(
+        Ctx, ProfileFileName,
+        "Function entry profiles are not yet supported for optimization",
+        DS_Error);
     return false;
   }
 
@@ -2335,12 +2333,11 @@ static bool annotateAllFunctions(
     // Only set when there is no Attribute::Hot set by the user. For Hot
     // attribute, user's annotation has the precedence over the profile.
     if (F->hasFnAttribute(Attribute::Hot)) {
-      auto &Ctx = M.getContext();
-      std::string Msg = std::string("Function ") + F->getName().str() +
-                        std::string(" is annotated as a hot function but"
-                                    " the profile is cold");
-      Ctx.diagnose(
-          DiagnosticInfoPGOProfile(M.getName().data(), Msg, DS_Warning));
+      emitDiagnostic(M.getContext(), M.getName(),
+                     Twine("Function ") + F->getName().str() +
+                         Twine(" is annotated as a hot function but"
+                               " the profile is cold"),
+                     DS_Warning);
       continue;
     }
     F->addFnAttr(Attribute::Cold);
diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp
index 7074376feed9c..9d195a86a5d76 100644
--- a/llvm/lib/Transforms/Utils/MisExpect.cpp
+++ b/llvm/lib/Transforms/Utils/MisExpect.cpp
@@ -100,18 +100,17 @@ Instruction *getInstCondition(Instruction *I) {
 void emitMisexpectDiagnostic(Instruction *I, LLVMContext &Ctx,
                              uint64_t ProfCount, uint64_t TotalCount) {
   double PercentageCorrect = (double)ProfCount / TotalCount;
-  auto PerString =
+  SmallString<32> PerString =
       formatv("{0:P} ({1} / {2})", PercentageCorrect, ProfCount, TotalCount);
-  auto RemStr = formatv(
+  SmallString<128> RemStr = formatv(
       "Potential performance regression from use of the llvm.expect intrinsic: "
       "Annotation was correct on {0} of profiled executions.",
       PerString);
-  Twine Msg(PerString);
   Instruction *Cond = getInstCondition(I);
   if (isMisExpectDiagEnabled(Ctx))
-    Ctx.diagnose(DiagnosticInfoMisExpect(Cond, Msg));
+    Ctx.diagnose(DiagnosticInfoMisExpect(Cond, PerString));
   OptimizationRemarkEmitter ORE(I->getParent()->getParent());
-  ORE.emit(OptimizationRemark(DEBUG_TYPE, "misexpect", Cond) << RemStr.str());
+  ORE.emit(OptimizationRemark(DEBUG_TYPE, "misexpect", Cond) << RemStr);
 }
 
 } // namespace



More information about the llvm-commits mailing list