[PATCH] D78859: [IR] Use map for string attributes (NFC)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: tejohnson, arsenm, wenlei.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.

Attributes (whether enum or string) are currently stored as a simple list. Enum attributes additionally use a bitset to allow quickly determining whether an attribute is set. String attributes on the other hand require a scan of the full list. As functions tend to have a lot of string attributes (at least when clang is used), this is a noticeable performance issue.

This patch adds an additional name => attribute map to the AttributeSetNode, which allows querying string attributes quickly. This results in a 3% reduction in instructions retired <http://llvm-compile-time-tracker.com/compare.php?from=367229e100eca714276253bf95a0dd3d084a9624&to=0cc4958dd2dda646abda6a7b01aeb6b50b601e6b&stat=instructions>. Changes to memory usage seem to be in the noise (attribute sets are uniqued, and we don't tend to have more than a few dozen or hundred unique attribute sets, so adding an extra map does not have a visible cost.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78859

Files:
  lib/IR/AttributeImpl.h
  lib/IR/Attributes.cpp


Index: lib/IR/Attributes.cpp
===================================================================
--- lib/IR/Attributes.cpp
+++ lib/IR/Attributes.cpp
@@ -770,7 +770,9 @@
                 "Too many attributes");
 
   for (const auto &I : *this) {
-    if (!I.isStringAttribute()) {
+    if (I.isStringAttribute()) {
+      StringAttrs.insert({ I.getKindAsString(), I });
+    } else {
       Attribute::AttrKind Kind = I.getKindAsEnum();
       AvailableAttrs[Kind / 8] |= 1ULL << (Kind % 8);
     }
@@ -857,10 +859,7 @@
 }
 
 bool AttributeSetNode::hasAttribute(StringRef Kind) const {
-  for (const auto &I : *this)
-    if (I.hasAttribute(Kind))
-      return true;
-  return false;
+  return StringAttrs.count(Kind);
 }
 
 Attribute AttributeSetNode::getAttribute(Attribute::AttrKind Kind) const {
@@ -873,10 +872,7 @@
 }
 
 Attribute AttributeSetNode::getAttribute(StringRef Kind) const {
-  for (const auto &I : *this)
-    if (I.hasAttribute(Kind))
-      return I;
-  return {};
+  return StringAttrs.lookup(Kind);
 }
 
 MaybeAlign AttributeSetNode::getAlignment() const {
Index: lib/IR/AttributeImpl.h
===================================================================
--- lib/IR/AttributeImpl.h
+++ lib/IR/AttributeImpl.h
@@ -16,6 +16,7 @@
 #define LLVM_LIB_IR_ATTRIBUTEIMPL_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Attributes.h"
@@ -181,6 +182,8 @@
   /// Bitset with a bit for each available attribute Attribute::AttrKind.
   uint8_t AvailableAttrs[12] = {};
 
+  DenseMap<StringRef, Attribute> StringAttrs;
+
   AttributeSetNode(ArrayRef<Attribute> Attrs);
 
 public:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78859.260079.patch
Type: text/x-patch
Size: 1693 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200425/58050875/attachment.bin>