[BOLT] Build CFG for non-simple functions in aggregation mode

[aaupov]

Extend #128253 by allowing all non-simple functions be processed in
aggregation mode, not just those with multiple entry points.

The CFG is needed for getFallthroughsInTrace to convert profile into
YAML/fdata. We're primarily interested in basic block boundaries, with
some of them could be incorrectly identified for non-simple functions.
However, this is benign in aggregation mode, in the worst case resulting
in stale profile for such functions.

Test Plan: entry-point-fallthru.s

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Extend #128253 by allowing all non-simple functions be processed in
aggregation mode, not just those with multiple entry points.

Test Plan: entry-point-fallthru.s

---

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

3 Files Affected:

• (modified) bolt/lib/Core/BinaryFunction.cpp (+2-3)
• (modified) bolt/lib/Profile/DataAggregator.cpp (+1-1)
• (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+2-1)

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index ff5eb5cf6e1eb..161733d75a2ab 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1754,8 +1754,7 @@ void BinaryFunction::postProcessEntryPoints() {
     // In non-relocation mode there's potentially an external undetectable
     // reference to the entry point and hence we cannot move this entry
     // point. Optimizing without moving could be difficult.
-    // In aggregation, register any known entry points for CFG construction.
-    if (!BC.HasRelocations && !opts::AggregateOnly)
+    if (!BC.HasRelocations)
       setSimple(false);
 
     const uint32_t Offset = KV.first;
@@ -2084,7 +2083,7 @@ void BinaryFunction::recomputeLandingPads() {
 Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
   auto &MIB = BC.MIB;
 
-  if (!isSimple()) {
+  if (!isSimple() && !opts::AggregateOnly) {
     assert(!BC.HasRelocations &&
            "cannot process file with non-simple function in relocs mode");
     return createNonFatalBOLTError("");
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index d20626bd5062f..dc0f91030bc18 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -871,7 +871,7 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,
 
   BinaryContext &BC = BF.getBinaryContext();
 
-  if (!BF.isSimple())
+  if (BF.empty())
     return std::nullopt;
 
   assert(BF.hasCFG() && "can only record traces in CFG state");
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 70a9f084f009b..ae3e0b9ddce38 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -3438,7 +3438,8 @@ void RewriteInstance::buildFunctionsCFG() {
       };
 
   ParallelUtilities::PredicateTy SkipPredicate = [&](const BinaryFunction &BF) {
-    return !shouldDisassemble(BF) || !BF.isSimple();
+    // Construct CFG for non-simple functions in aggregation mode.
+    return !(shouldDisassemble(BF) && (BF.isSimple() || opts::AggregateOnly));
   };
 
   ParallelUtilities::runOnEachFunctionWithUniqueAllocId(

[maksfb]

I think we need to clearly mark functions that may have incorrect basic block boundaries. Otherwise LGTM.

[aaupov]

I think we need to clearly mark functions that may have incorrect basic block boundaries.

What kind of marking do you have in mind, a new BinaryFunction attribute e.g. MayHaveInexactBBBounds? Or make IsSimple an enum like {Simple, Inexact BBs, Non-simple}?