[llvm] [BOLT][NFC] Simplify DataAggregator::getFallthroughsInTrace (PR #90752)

via llvm-commits llvm-commits at lists.llvm.org
Wed May 1 10:21:24 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

<details>
<summary>Changes</summary>

Merge DataAggregator::recordTrace into getFallthroughsInTrace.


---
Full diff: https://github.com/llvm/llvm-project/pull/90752.diff


2 Files Affected:

- (modified) bolt/include/bolt/Profile/DataAggregator.h (+1-7) 
- (modified) bolt/lib/Profile/DataAggregator.cpp (+13-23) 


``````````diff
diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h
index 84f76caae9dbb0..f2fa59bcaa1a3a 100644
--- a/bolt/include/bolt/Profile/DataAggregator.h
+++ b/bolt/include/bolt/Profile/DataAggregator.h
@@ -198,14 +198,8 @@ class DataAggregator : public DataReader {
   /// A trace is region of code executed between two LBR entries supplied in
   /// execution order.
   ///
-  /// Return true if the trace is valid, false otherwise.
-  bool
-  recordTrace(BinaryFunction &BF, const LBREntry &First, const LBREntry &Second,
-              uint64_t Count,
-              SmallVector<std::pair<uint64_t, uint64_t>, 16> &Branches) const;
-
   /// Return a vector of offsets corresponding to a trace in a function
-  /// (see recordTrace() above).
+  /// if the trace is valid, std::nullopt otherwise.
   std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
   getFallthroughsInTrace(BinaryFunction &BF, const LBREntry &First,
                          const LBREntry &Second, uint64_t Count = 1) const;
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 70e324cc0165bb..5108392c824c10 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -861,14 +861,17 @@ bool DataAggregator::doTrace(const LBREntry &First, const LBREntry &Second,
   return true;
 }
 
-bool DataAggregator::recordTrace(
-    BinaryFunction &BF, const LBREntry &FirstLBR, const LBREntry &SecondLBR,
-    uint64_t Count,
-    SmallVector<std::pair<uint64_t, uint64_t>, 16> &Branches) const {
+std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
+DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,
+                                       const LBREntry &FirstLBR,
+                                       const LBREntry &SecondLBR,
+                                       uint64_t Count) const {
+  SmallVector<std::pair<uint64_t, uint64_t>, 16> Branches;
+
   BinaryContext &BC = BF.getBinaryContext();
 
   if (!BF.isSimple())
-    return false;
+    return std::nullopt;
 
   assert(BF.hasCFG() && "can only record traces in CFG state");
 
@@ -877,13 +880,13 @@ bool DataAggregator::recordTrace(
   const uint64_t To = SecondLBR.From - BF.getAddress();
 
   if (From > To)
-    return false;
+    return std::nullopt;
 
   const BinaryBasicBlock *FromBB = BF.getBasicBlockContainingOffset(From);
   const BinaryBasicBlock *ToBB = BF.getBasicBlockContainingOffset(To);
 
   if (!FromBB || !ToBB)
-    return false;
+    return std::nullopt;
 
   // Adjust FromBB if the first LBR is a return from the last instruction in
   // the previous block (that instruction should be a call).
@@ -907,7 +910,7 @@ bool DataAggregator::recordTrace(
   // within the same basic block, e.g. when two call instructions are in the
   // same block. In this case we skip the processing.
   if (FromBB == ToBB)
-    return true;
+    return Branches;
 
   // Process blocks in the original layout order.
   BinaryBasicBlock *BB = BF.getLayout().getBlock(FromBB->getIndex());
@@ -921,7 +924,7 @@ bool DataAggregator::recordTrace(
       LLVM_DEBUG(dbgs() << "no fall-through for the trace:\n"
                         << "  " << FirstLBR << '\n'
                         << "  " << SecondLBR << '\n');
-      return false;
+      return std::nullopt;
     }
 
     const MCInst *Instr = BB->getLastNonPseudoInstr();
@@ -945,20 +948,7 @@ bool DataAggregator::recordTrace(
     BI.Count += Count;
   }
 
-  return true;
-}
-
-std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
-DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,
-                                       const LBREntry &FirstLBR,
-                                       const LBREntry &SecondLBR,
-                                       uint64_t Count) const {
-  SmallVector<std::pair<uint64_t, uint64_t>, 16> Res;
-
-  if (!recordTrace(BF, FirstLBR, SecondLBR, Count, Res))
-    return std::nullopt;
-
-  return Res;
+  return Branches;
 }
 
 bool DataAggregator::recordEntry(BinaryFunction &BF, uint64_t To, bool Mispred,

``````````

</details>


https://github.com/llvm/llvm-project/pull/90752


More information about the llvm-commits mailing list