[PATCH] D43520: [ThinLTO] Represent relative BF using a scaled representation .

[Easwaran Raman via Phabricator via llvm-commits]

eraman updated this revision to Diff 135178.
eraman marked 2 inline comments as done.
eraman added a comment.

Address review comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D43520

Files:
  include/llvm/IR/ModuleSummaryIndex.h
  lib/Analysis/ModuleSummaryAnalysis.cpp
  test/Bitcode/thinlto-function-summary-callgraph-relbf.ll


Index: test/Bitcode/thinlto-function-summary-callgraph-relbf.ll
===================================================================
--- test/Bitcode/thinlto-function-summary-callgraph-relbf.ll
+++ test/Bitcode/thinlto-function-summary-callgraph-relbf.ll
@@ -11,7 +11,7 @@
 ; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
 ; CHECK-NEXT:    <VERSION
 ; See if the call to func is registered.
-; CHECK-NEXT:    <PERMODULE_RELBF {{.*}} op4=1 {{.*}} op7=1
+; CHECK-NEXT:    <PERMODULE_RELBF {{.*}} op4=1 {{.*}} op7=256
 ; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 ; CHECK: <STRTAB_BLOCK
 ; CHECK-NEXT: blob data = 'undefinedglobmainfunc{{.*}}'
Index: lib/Analysis/ModuleSummaryAnalysis.cpp
===================================================================
--- lib/Analysis/ModuleSummaryAnalysis.cpp
+++ lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -279,17 +279,9 @@
         // Add the relative block frequency to CalleeInfo if there is no profile
         // information.
         if (BFI != nullptr && Hotness == CalleeInfo::HotnessType::Unknown) {
-          auto BBFreq = BFI->getBlockFreq(&BB).getFrequency();
-          // FIXME: This might need some scaling to prevent BBFreq values from
-          // being rounded down to 0.
-          auto EntryFreq = BFI->getEntryFreq();
-          // Block frequencies can be directly set for a block and so we need to
-          // handle the case of entry frequency being 0.
-          if (EntryFreq)
-            BBFreq /= EntryFreq;
-          else
-            BBFreq = 0;
-          ValueInfo.updateRelBlockFreq(BBFreq);
+          uint64_t BBFreq = BFI->getBlockFreq(&BB).getFrequency();
+          uint64_t EntryFreq = BFI->getEntryFreq();
+          ValueInfo.updateRelBlockFreq(BBFreq, EntryFreq);
         }
       } else {
         // Skip inline assembly calls.
Index: include/llvm/IR/ModuleSummaryIndex.h
===================================================================
--- include/llvm/IR/ModuleSummaryIndex.h
+++ include/llvm/IR/ModuleSummaryIndex.h
@@ -26,6 +26,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/ScaledNumber.h"
 #include <algorithm>
 #include <array>
 #include <cassert>
@@ -59,7 +60,11 @@
   // The size of the bit-field might need to be adjusted if more values are
   // added to HotnessType enum.
   uint32_t Hotness : 3;
+
+  /// The value stored in RelBlockFreq has to be interpreted as the digits of
+  /// a scaled number with a scale of \p -ScaleShift.
   uint32_t RelBlockFreq : 29;
+  static constexpr int32_t ScaleShift = 8;
   static constexpr uint64_t MaxRelBlockFreq = (1 << 29) - 1;
 
   CalleeInfo()
@@ -73,10 +78,20 @@
 
   HotnessType getHotness() const { return HotnessType(Hotness); }
 
-  // When there are multiple edges between the same (caller, callee) pair, the
-  // relative block frequencies are summed up.
-  void updateRelBlockFreq(uint64_t RBF) {
-    uint64_t Sum = SaturatingAdd<uint64_t>(RelBlockFreq, RBF);
+  /// Update \p RelBlockFreq from \p BlockFreq and \p EntryFreq
+  ///
+  /// BlockFreq is divided by EntryFreq and added to RelBlockFreq. To represent
+  /// fractional values, the result is represented as a fixed point number with
+  /// scale of -ScaleShift.
+  void updateRelBlockFreq(uint64_t BlockFreq, uint64_t EntryFreq) {
+    if (EntryFreq == 0)
+      return;
+    using Scaled64 = ScaledNumber<uint64_t>;
+    Scaled64 Temp(BlockFreq, ScaleShift);
+    Temp /= Scaled64::get(EntryFreq);
+
+    uint64_t Sum =
+        SaturatingAdd<uint64_t>(Temp.toInt<uint64_t>(), RelBlockFreq);
     Sum = std::min(Sum, uint64_t(MaxRelBlockFreq));
     RelBlockFreq = static_cast<uint32_t>(Sum);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43520.135178.patch
Type: text/x-patch
Size: 3690 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180221/918baa9c/attachment.bin>