[llvm] [SelectionDAG] Stop storing EVTs in a function scoped static std::set. (PR #118715)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 4 23:10:53 PST 2024


https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/118715

>From 267aa0fecf0da4ccd7ffddfd39316d3f9ec2f255 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 24 Apr 2024 13:34:23 -0700
Subject: [PATCH 1/2] [SelectionDAG] Stop storing EVTs in a function scoped
 static std::set.

EVTs potentially contain a Type * that points into memory owned by an
LLVMContext. Storing them in a function scoped static means they may
outlive the LLVMContext they point to.

This std::set is used to unique single element VT lists
containing a single extended EVT. Single element VT list with a
simple EVT are uniqued by a separate cache indexed by the
MVT::SimpleValueType enum. VT lists with more than one element are
uniqued by a FoldingSet owned by the SelectionDAG object.

This patch removes the single element extended EVT cache and puts them
in the FoldingSet instead. This ensures they won't outlive the LLVMContext.

There is a bit of a compile time hit for this. Data from a measurement
last month https://llvm-compile-time-tracker.com/compare.php?from=38b0e1c939e818564019bb5bff95a0f1abbf9d19&to=4bd9484a0aa9d3ed55b50536b6810e8909533b4b&stat=instructions:ua

I wonder if it may be because types like i24, i40, i48, i56 can show
up sometimes, but they aren't MVTs.

Fixes #88233
---
 llvm/include/llvm/CodeGen/SelectionDAGNodes.h |  4 +--
 .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 29 ++++++++++++-------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
index 677b59e0c8fbeb..61f3c6329efce8 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -664,7 +664,7 @@ END_TWO_BYTE_PACK()
   DebugLoc debugLoc;
 
   /// Return a pointer to the specified value type.
-  static const EVT *getValueTypeList(EVT VT);
+  static const EVT *getValueTypeList(MVT VT);
 
   /// Index in worklist of DAGCombiner, or negative if the node is not in the
   /// worklist. -1 = not in worklist; -2 = not in worklist, but has already been
@@ -1124,7 +1124,7 @@ END_TWO_BYTE_PACK()
   void addUse(SDUse &U) { U.addToList(&UseList); }
 
 protected:
-  static SDVTList getSDVTList(EVT VT) {
+  static SDVTList getSDVTList(MVT VT) {
     SDVTList Ret = { getValueTypeList(VT), 1 };
     return Ret;
   }
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 182529123ec6d8..4cfebd39e07eae 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -10623,7 +10623,22 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, SDVTList VTList,
 }
 
 SDVTList SelectionDAG::getVTList(EVT VT) {
-  return makeVTList(SDNode::getValueTypeList(VT), 1);
+  if (!VT.isExtended())
+    return makeVTList(SDNode::getValueTypeList(VT.getSimpleVT()), 1);
+
+  FoldingSetNodeID ID;
+  ID.AddInteger(1U);
+  ID.AddInteger(VT.getRawBits());
+
+  void *IP = nullptr;
+  SDVTListNode *Result = VTListMap.FindNodeOrInsertPos(ID, IP);
+  if (!Result) {
+    EVT *Array = Allocator.Allocate<EVT>(1);
+    Array[0] = VT;
+    Result = new (Allocator) SDVTListNode(ID.Intern(Allocator), Array, 1);
+    VTListMap.InsertNode(Result, IP);
+  }
+  return Result->getSDVTList();
 }
 
 SDVTList SelectionDAG::getVTList(EVT VT1, EVT VT2) {
@@ -12383,17 +12398,11 @@ namespace {
 
 /// getValueTypeList - Return a pointer to the specified value type.
 ///
-const EVT *SDNode::getValueTypeList(EVT VT) {
-  static std::set<EVT, EVT::compareRawBits> EVTs;
+const EVT *SDNode::getValueTypeList(MVT VT) {
   static EVTArray SimpleVTArray;
-  static sys::SmartMutex<true> VTMutex;
 
-  if (VT.isExtended()) {
-    sys::SmartScopedLock<true> Lock(VTMutex);
-    return &(*EVTs.insert(VT).first);
-  }
-  assert(VT.getSimpleVT() < MVT::VALUETYPE_SIZE && "Value type out of range!");
-  return &SimpleVTArray.VTs[VT.getSimpleVT().SimpleTy];
+  assert(VT < MVT::VALUETYPE_SIZE && "Value type out of range!");
+  return &SimpleVTArray.VTs[VT.SimpleTy];
 }
 
 /// hasNUsesOfValue - Return true if there are exactly NUSES uses of the

>From c001489a8bd21f73a9c3d4397b29143c94fce129 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 4 Dec 2024 23:10:34 -0800
Subject: [PATCH 2/2] fixup! Move std::set into SelectionDAG instead of reusing
 FoldingSet.

---
 llvm/include/llvm/CodeGen/SelectionDAG.h       |  4 ++++
 llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 14 +-------------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index 2e3507386df309..e97e01839f73b4 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -44,6 +44,7 @@
 #include <cstdint>
 #include <functional>
 #include <map>
+#include <set>
 #include <string>
 #include <tuple>
 #include <utility>
@@ -247,6 +248,9 @@ class SelectionDAG {
   BlockFrequencyInfo *BFI = nullptr;
   MachineModuleInfo *MMI = nullptr;
 
+  /// Extended EVTs used for single value VTLists.
+  std::set<EVT, EVT::compareRawBits> EVTs;
+
   /// List of non-single value types.
   FoldingSet<SDVTListNode> VTListMap;
 
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 4cfebd39e07eae..1b39b75d2687f5 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -10626,19 +10626,7 @@ SDVTList SelectionDAG::getVTList(EVT VT) {
   if (!VT.isExtended())
     return makeVTList(SDNode::getValueTypeList(VT.getSimpleVT()), 1);
 
-  FoldingSetNodeID ID;
-  ID.AddInteger(1U);
-  ID.AddInteger(VT.getRawBits());
-
-  void *IP = nullptr;
-  SDVTListNode *Result = VTListMap.FindNodeOrInsertPos(ID, IP);
-  if (!Result) {
-    EVT *Array = Allocator.Allocate<EVT>(1);
-    Array[0] = VT;
-    Result = new (Allocator) SDVTListNode(ID.Intern(Allocator), Array, 1);
-    VTListMap.InsertNode(Result, IP);
-  }
-  return Result->getSDVTList();
+  return makeVTList(&(*EVTs.insert(VT).first), 1);
 }
 
 SDVTList SelectionDAG::getVTList(EVT VT1, EVT VT2) {



More information about the llvm-commits mailing list