[clang-tools-extra] [clang] [llvm] [BOLT] Use direct storage for Label annotations. NFCI. (PR #70147)

Maksim Panchenko via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 6 14:07:21 PST 2023


https://github.com/maksfb updated https://github.com/llvm/llvm-project/pull/70147

>From 0860630c42430aa8c571ebe8931fd7a009ade560 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Thu, 19 Oct 2023 19:34:44 -0700
Subject: [PATCH 1/2] [BOLT] Use direct storage for Label annotations. NFCI.

Store the Label annotation directly in the operand and avoid the
extra allocation and indirection overheads associated with
MCSimpleAnnotation.
---
 bolt/include/bolt/Core/MCPlusBuilder.h |  4 +++-
 bolt/lib/Core/MCPlusBuilder.cpp        | 16 +++++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index d136c627bc5cc10..5ff1195facf7d34 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -1185,7 +1185,7 @@ class MCPlusBuilder {
 
   /// Set the label of \p Inst. This label will be emitted right before \p Inst
   /// is emitted to MCStreamer.
-  bool setLabel(MCInst &Inst, MCSymbol *Label, AllocatorIdTy AllocatorId = 0);
+  bool setLabel(MCInst &Inst, MCSymbol *Label);
 
   /// Return MCSymbol that represents a target of this instruction at a given
   /// operand number \p OpNum. If there's no symbol associated with
@@ -1820,6 +1820,8 @@ class MCPlusBuilder {
   const ValueType &addAnnotation(MCInst &Inst, unsigned Index,
                                  const ValueType &Val,
                                  AllocatorIdTy AllocatorId = 0) {
+    assert(Index >= MCPlus::MCAnnotation::kGeneric &&
+           "Generic annotation type expected.");
     assert(!hasAnnotation(Inst, Index));
     AnnotationAllocator &Allocator = getAnnotationAllocator(AllocatorId);
     auto *A = new (Allocator.ValueAllocator)
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index 036fcf8b3e27fd9..2dbb85d7387dd02 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -269,15 +269,17 @@ bool MCPlusBuilder::clearOffset(MCInst &Inst) {
 }
 
 std::optional<MCSymbol *> MCPlusBuilder::getLabel(const MCInst &Inst) const {
-  if (auto Label = tryGetAnnotationAs<MCSymbol *>(Inst, MCAnnotation::kLabel))
-    return *Label;
-  return std::nullopt;
+  std::optional<int64_t> Label =
+      getAnnotationOpValue(Inst, MCAnnotation::kLabel);
+  if (!Label)
+    return std::nullopt;
+
+  return reinterpret_cast<MCSymbol *>(*Label);
 }
 
-bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label,
-                             AllocatorIdTy AllocatorId) {
-  getOrCreateAnnotationAs<MCSymbol *>(Inst, MCAnnotation::kLabel, AllocatorId) =
-      Label;
+bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) {
+  setAnnotationOpValue(Inst, MCAnnotation::kLabel,
+                       reinterpret_cast<int64_t>(Label));
   return true;
 }
 

>From 97348c75a17a46ecd74c7a1f61de4e02c29e3686 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Mon, 6 Nov 2023 14:05:36 -0800
Subject: [PATCH 2/2] Make setLabel() const

---
 bolt/include/bolt/Core/MCPlusBuilder.h | 2 +-
 bolt/lib/Core/MCPlusBuilder.cpp        | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index 1bdcd43845ec4dd..66d79c753d69977 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -1181,7 +1181,7 @@ class MCPlusBuilder {
 
   /// Set the label of \p Inst. This label will be emitted right before \p Inst
   /// is emitted to MCStreamer.
-  bool setLabel(MCInst &Inst, MCSymbol *Label);
+  bool setLabel(MCInst &Inst, MCSymbol *Label) const;
 
   /// Return MCSymbol that represents a target of this instruction at a given
   /// operand number \p OpNum. If there's no symbol associated with
diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp
index deb3a8728acc591..8c8ebe7d9830d26 100644
--- a/bolt/lib/Core/MCPlusBuilder.cpp
+++ b/bolt/lib/Core/MCPlusBuilder.cpp
@@ -268,12 +268,12 @@ bool MCPlusBuilder::clearOffset(MCInst &Inst) const {
 
 MCSymbol *MCPlusBuilder::getLabel(const MCInst &Inst) const {
   if (std::optional<int64_t> Label =
-      getAnnotationOpValue(Inst, MCAnnotation::kLabel))
+          getAnnotationOpValue(Inst, MCAnnotation::kLabel))
     return reinterpret_cast<MCSymbol *>(*Label);
   return nullptr;
 }
 
-bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) {
+bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) const {
   setAnnotationOpValue(Inst, MCAnnotation::kLabel,
                        reinterpret_cast<int64_t>(Label));
   return true;



More information about the cfe-commits mailing list