[llvm] [BOLT] Fix double conversion in CacheMetrics (PR #75253)

[via llvm-commits]

https://github.com/spupyrev updated https://github.com/llvm/llvm-project/pull/75253

>From 5f9322128486d2e92bab99e1a1e9b0f510305c47 Mon Sep 17 00:00:00 2001
From: spupyrev <spupyrev at fb.com>
Date: Tue, 12 Dec 2023 14:50:13 -0800
Subject: [PATCH 1/3] [BOLT] Fixing double conversion in CacheMetrics

---
 bolt/lib/Passes/CacheMetrics.cpp | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/bolt/lib/Passes/CacheMetrics.cpp b/bolt/lib/Passes/CacheMetrics.cpp
index ba3a2a5f685f38..20055dc77c3df0 100644
--- a/bolt/lib/Passes/CacheMetrics.cpp
+++ b/bolt/lib/Passes/CacheMetrics.cpp
@@ -14,22 +14,18 @@
 #include "bolt/Passes/CacheMetrics.h"
 #include "bolt/Core/BinaryBasicBlock.h"
 #include "bolt/Core/BinaryFunction.h"
-#include "llvm/Support/CommandLine.h"
 #include <unordered_map>
 
 using namespace llvm;
 using namespace bolt;
 
-namespace opts {
-
-extern cl::OptionCategory BoltOptCategory;
+namespace {
 
-extern cl::opt<unsigned> ITLBPageSize;
-extern cl::opt<unsigned> ITLBEntries;
+/// The size of an i-tlb cache page.
+constexpr unsigned ITLBPageSize = 4096;
 
-} // namespace opts
-
-namespace {
+/// The number of entries in the i-tlb cache.
+constexpr unsigned ITLBEntries = 16;
 
 /// Initialize and return a position map for binary basic blocks
 void extractBasicBlockInfo(
@@ -133,9 +129,6 @@ double expectedCacheHitRatio(
     const std::vector<BinaryFunction *> &BinaryFunctions,
     const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBAddr,
     const std::unordered_map<BinaryBasicBlock *, uint64_t> &BBSize) {
-
-  const double PageSize = opts::ITLBPageSize;
-  const uint64_t CacheEntries = opts::ITLBEntries;
   std::unordered_map<const BinaryFunction *, Predecessors> Calls =
       extractFunctionCalls(BinaryFunctions);
   // Compute 'hotness' of the functions
@@ -155,7 +148,7 @@ double expectedCacheHitRatio(
   for (BinaryFunction *BF : BinaryFunctions) {
     if (BF->getLayout().block_empty())
       continue;
-    double Page = BBAddr.at(BF->getLayout().block_front()) / PageSize;
+    uint64_t Page = BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
     PageSamples[Page] += FunctionSamples.at(BF);
   }
 
@@ -166,15 +159,15 @@ double expectedCacheHitRatio(
     if (BF->getLayout().block_empty() || FunctionSamples.at(BF) == 0.0)
       continue;
     double Samples = FunctionSamples.at(BF);
-    double Page = BBAddr.at(BF->getLayout().block_front()) / PageSize;
+    uint64_t Page = BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
     // The probability that the page is not present in the cache
-    double MissProb = pow(1.0 - PageSamples[Page] / TotalSamples, CacheEntries);
+    double MissProb = pow(1.0 - PageSamples[Page] / TotalSamples, ITLBEntries);
 
     // Processing all callers of the function
     for (std::pair<BinaryFunction *, uint64_t> Pair : Calls[BF]) {
       BinaryFunction *SrcFunction = Pair.first;
-      double SrcPage =
-          BBAddr.at(SrcFunction->getLayout().block_front()) / PageSize;
+      uint64_t SrcPage =
+          BBAddr.at(SrcFunction->getLayout().block_front()) / ITLBPageSize;
       // Is this a 'long' or a 'short' call?
       if (Page != SrcPage) {
         // This is a miss

>From 33972d1ccb77e93f93232aa20ce602c5f3e46447 Mon Sep 17 00:00:00 2001
From: spupyrev <spupyrev at fb.com>
Date: Wed, 13 Dec 2023 08:29:58 -0800
Subject: [PATCH 2/3] adding const

---
 bolt/lib/Passes/CacheMetrics.cpp | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/bolt/lib/Passes/CacheMetrics.cpp b/bolt/lib/Passes/CacheMetrics.cpp
index 20055dc77c3df0..354014b69c728b 100644
--- a/bolt/lib/Passes/CacheMetrics.cpp
+++ b/bolt/lib/Passes/CacheMetrics.cpp
@@ -148,7 +148,8 @@ double expectedCacheHitRatio(
   for (BinaryFunction *BF : BinaryFunctions) {
     if (BF->getLayout().block_empty())
       continue;
-    uint64_t Page = BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
+    const uint64_t Page =
+        BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
     PageSamples[Page] += FunctionSamples.at(BF);
   }
 
@@ -159,14 +160,16 @@ double expectedCacheHitRatio(
     if (BF->getLayout().block_empty() || FunctionSamples.at(BF) == 0.0)
       continue;
     double Samples = FunctionSamples.at(BF);
-    uint64_t Page = BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
+    const uint64_t Page =
+        BBAddr.at(BF->getLayout().block_front()) / ITLBPageSize;
     // The probability that the page is not present in the cache
-    double MissProb = pow(1.0 - PageSamples[Page] / TotalSamples, ITLBEntries);
+    const double MissProb =
+        pow(1.0 - PageSamples[Page] / TotalSamples, ITLBEntries);
 
     // Processing all callers of the function
     for (std::pair<BinaryFunction *, uint64_t> Pair : Calls[BF]) {
       BinaryFunction *SrcFunction = Pair.first;
-      uint64_t SrcPage =
+      const uint64_t SrcPage =
           BBAddr.at(SrcFunction->getLayout().block_front()) / ITLBPageSize;
       // Is this a 'long' or a 'short' call?
       if (Page != SrcPage) {

>From c0350fba3090cb4ae4924b66a0c6ae3f6c37fa8f Mon Sep 17 00:00:00 2001
From: spupyrev <spupyrev at fb.com>
Date: Fri, 12 Jan 2024 09:57:27 -0800
Subject: [PATCH 3/3] modified a comment

---
 bolt/lib/Passes/CacheMetrics.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bolt/lib/Passes/CacheMetrics.cpp b/bolt/lib/Passes/CacheMetrics.cpp
index 354014b69c728b..f6708997a43350 100644
--- a/bolt/lib/Passes/CacheMetrics.cpp
+++ b/bolt/lib/Passes/CacheMetrics.cpp
@@ -21,10 +21,11 @@ using namespace bolt;
 
 namespace {
 
-/// The size of an i-tlb cache page.
+/// The following constants are used to estimate the number of i-TLB cache
+/// misses for a given code layout. Empirically the values result in high
+/// correlations between the estimations and the perf measurements.
+/// The constants do not affect the code layout algorithms.
 constexpr unsigned ITLBPageSize = 4096;
-
-/// The number of entries in the i-tlb cache.
 constexpr unsigned ITLBEntries = 16;
 
 /// Initialize and return a position map for binary basic blocks