Skip to content

Conversation

@aaupov
Copy link
Contributor

@aaupov aaupov commented Jun 7, 2025

Parsed branches and fall-throughs are validated in doBranch and
doTrace respectively. Simplify parseLBRSample by omitting the
validation. This also speeds up perf data processing as checks are only
done once for aggregated branches/fall-throughs and not individual LBR
entries.

Since invalid/external addresses are no longer sanitized during parsing,
sanitize them in doBranch.

Test Plan: NFC

aaupov added 3 commits June 7, 2025 14:48
Created using spr 1.3.4
Created using spr 1.3.4
@aaupov aaupov marked this pull request as ready for review June 7, 2025 22:11
@llvmbot llvmbot added the BOLT label Jun 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 7, 2025

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Parsed branches and fall-throughs are validated in doBranch and
doTrace respectively. Simplify parseLBRSample by omitting the
validation. This also speeds up perf data processing as checks are only
done once for aggregated branches/fall-throughs and not individual LBR
entries.

Test Plan: NFC


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

1 Files Affected:

  • (modified) bolt/lib/Profile/DataAggregator.cpp (+5-43)
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index b1172fd13bc72..addff196f4f5b 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -1425,54 +1425,16 @@ void DataAggregator::parseLBRSample(const PerfBranchSample &Sample,
       const uint64_t TraceTo = NextLBR->From;
       const BinaryFunction *TraceBF =
           getBinaryFunctionContainingAddress(TraceFrom);
-      if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive) {
-        FTInfo &Info = FallthroughLBRs[Trace(TraceFrom, TraceTo)];
+      FTInfo &Info = FallthroughLBRs[Trace(TraceFrom, TraceTo)];
+      if (TraceBF && TraceBF->containsAddress(LBR.From))
         ++Info.InternCount;
-      } else if (TraceBF && TraceBF->containsAddress(TraceTo)) {
-        FTInfo &Info = FallthroughLBRs[Trace(TraceFrom, TraceTo)];
-        if (TraceBF->containsAddress(LBR.From))
-          ++Info.InternCount;
-        else
-          ++Info.ExternCount;
-      } else {
-        const BinaryFunction *ToFunc =
-            getBinaryFunctionContainingAddress(TraceTo);
-        if (TraceBF && ToFunc) {
-          LLVM_DEBUG({
-            dbgs() << "Invalid trace starting in " << TraceBF->getPrintName()
-                   << formatv(" @ {0:x}", TraceFrom - TraceBF->getAddress())
-                   << formatv(" and ending @ {0:x}\n", TraceTo);
-          });
-          ++NumInvalidTraces;
-        } else {
-          LLVM_DEBUG({
-            dbgs() << "Out of range trace starting in "
-                   << (TraceBF ? TraceBF->getPrintName() : "None")
-                   << formatv(" @ {0:x}",
-                              TraceFrom - (TraceBF ? TraceBF->getAddress() : 0))
-                   << " and ending in "
-                   << (ToFunc ? ToFunc->getPrintName() : "None")
-                   << formatv(" @ {0:x}\n",
-                              TraceTo - (ToFunc ? ToFunc->getAddress() : 0));
-          });
-          ++NumLongRangeTraces;
-        }
-      }
+      else
+        ++Info.ExternCount;
       ++NumTraces;
     }
     NextLBR = &LBR;
 
-    // Record branches outside binary functions for heatmap.
-    if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive) {
-      TakenBranchInfo &Info = BranchLBRs[Trace(LBR.From, LBR.To)];
-      ++Info.TakenCount;
-      continue;
-    }
-    uint64_t From = getBinaryFunctionContainingAddress(LBR.From) ? LBR.From : 0;
-    uint64_t To = getBinaryFunctionContainingAddress(LBR.To) ? LBR.To : 0;
-    if (!From && !To)
-      continue;
-    TakenBranchInfo &Info = BranchLBRs[Trace(From, To)];
+    TakenBranchInfo &Info = BranchLBRs[Trace(LBR.From, LBR.To)];
     ++Info.TakenCount;
     Info.MispredCount += LBR.Mispred;
   }

aaupov added 3 commits June 7, 2025 20:32
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
aaupov added 2 commits June 8, 2025 17:44
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@aaupov aaupov changed the base branch from users/aaupov/spr/main.boltnfci-skip-validation-in-parselbrsample to main June 9, 2025 00:44
@aaupov aaupov merged commit 03bbd04 into main Jun 9, 2025
8 of 13 checks passed
@aaupov aaupov deleted the users/aaupov/spr/boltnfci-skip-validation-in-parselbrsample branch June 9, 2025 00:50
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
Parsed branches and fall-throughs are validated in `doBranch` and
`doTrace` respectively. Simplify parseLBRSample by omitting the
validation. This also speeds up perf data processing as checks are only
done once for aggregated branches/fall-throughs and not individual LBR
entries.

Since invalid/external addresses are no longer sanitized during parsing,
sanitize them in `doBranch`.

Test Plan: updated X86/pre-aggregated-perf.test
kaadam added a commit to kaadam/llvm-project that referenced this pull request Jun 19, 2025
The test could be simplified after llvm#143288 PR since
the validation phase is removed from parseLBRSample.
Now we can use branchLBRs container for the testing.
Formerly if Bolt was supplied with mock addresses, branchLBRs container
was empty due to validation phase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants