From 151142318c497feab0b66161c4ca8fe7017f0346 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks@fb.com>
Date: Thu, 1 Feb 2024 19:24:54 -0800
Subject: [PATCH] [BOLT][NFCI] Keep instruction annotations

We used to delete most instruction annotations before code emission. It
was done to release memory taken by annotations and to reduce overall
memory consumption. However, since the implementation of annotations
has moved to using existing instruction operands, the memory overhead
associated with them has reduced drastically. I measured that savings
are less than 0.5% on large binaries and processing time is just
slightly reduced if we keep them. Additionally, I plan to use
annotations in pre-emission passes for the Linux kernel rewriter.
---
 bolt/lib/Passes/BinaryPasses.cpp | 47 ++++++++++----------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 8505d37491415..bcb12272d1b46 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -582,6 +582,7 @@ bool CheckLargeFunctions::shouldOptimize(const BinaryFunction &BF) const {
 }
 
 void LowerAnnotations::runOnFunctions(BinaryContext &BC) {
+  // Convert GnuArgsSize annotations into CFIs.
   for (BinaryFunction *BF : BC.getAllBinaryFunctions()) {
     for (FunctionFragment &FF : BF->getLayout().fragments()) {
       // Reset at the start of the new fragment.
@@ -589,43 +590,23 @@ void LowerAnnotations::runOnFunctions(BinaryContext &BC) {
 
       for (BinaryBasicBlock *const BB : FF) {
         for (auto II = BB->begin(); II != BB->end(); ++II) {
-
-          // Convert GnuArgsSize annotations into CFIs.
-          if (BF->usesGnuArgsSize() && BC.MIB->isInvoke(*II)) {
-            const int64_t NewGnuArgsSize = BC.MIB->getGnuArgsSize(*II);
-            assert(NewGnuArgsSize >= 0 &&
-                   "Expected non-negative GNU_args_size.");
-            if (NewGnuArgsSize != CurrentGnuArgsSize) {
-              auto InsertII = BF->addCFIInstruction(
-                  BB, II,
-                  MCCFIInstruction::createGnuArgsSize(nullptr, NewGnuArgsSize));
-              CurrentGnuArgsSize = NewGnuArgsSize;
-              II = std::next(InsertII);
-            }
-          }
-
-          // Preserve selected annotations and strip the rest.
-          std::optional<uint32_t> Offset = BF->requiresAddressTranslation()
-                                               ? BC.MIB->getOffset(*II)
-                                               : std::nullopt;
-          std::optional<uint32_t> Size = BC.MIB->getSize(*II);
-          MCSymbol *Label = BC.MIB->getLabel(*II);
-
-          BC.MIB->stripAnnotations(*II);
-
-          if (Offset)
-            BC.MIB->setOffset(*II, *Offset);
-          if (Size)
-            BC.MIB->setSize(*II, *Size);
-          if (Label)
-            BC.MIB->setLabel(*II, Label);
+          if (!BF->usesGnuArgsSize() || !BC.MIB->isInvoke(*II))
+            continue;
+
+          const int64_t NewGnuArgsSize = BC.MIB->getGnuArgsSize(*II);
+          assert(NewGnuArgsSize >= 0 && "Expected non-negative GNU_args_size.");
+          if (NewGnuArgsSize == CurrentGnuArgsSize)
+            continue;
+
+          auto InsertII = BF->addCFIInstruction(
+              BB, II,
+              MCCFIInstruction::createGnuArgsSize(nullptr, NewGnuArgsSize));
+          CurrentGnuArgsSize = NewGnuArgsSize;
+          II = std::next(InsertII);
         }
       }
     }
   }
-
-  // Release all memory taken by annotations
-  BC.MIB->freeAnnotations();
 }
 
 // Check for dirty state in MCSymbol objects that might be a consequence