Skip to content

[BOLT] Drop blocks without profile in BAT YAML #107970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented Sep 10, 2024

Align BAT YAML (DataAggregator) to YAMLProfileWriter which drops blocks
without profile:

// Skip printing if there's no profile data for non-entry basic block.
// Include landing pads with non-zero execution count.
if (YamlBB.CallSites.empty() && !BB->isEntryPoint() &&
!(BB->isLandingPad() && BB->getKnownExecutionCount() != 0)) {
// Include blocks having successors or predecessors with positive counts.
uint64_t SuccessorExecCount = 0;
for (const BinaryBasicBlock::BinaryBranchInfo &BranchInfo :
BB->branch_info())
SuccessorExecCount += BranchInfo.Count;
uint64_t PredecessorExecCount = 0;
for (auto Pred : BB->predecessors())
PredecessorExecCount += Pred->getBranchInfo(*BB).Count;
if (!SuccessorExecCount && !PredecessorExecCount)
continue;
}

Test Plan: NFCI

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Align BAT YAML (DataAggregator) to YAMLProfileWriter which drops blocks
without profile:
https://github.com/llvm/llvm-project/blob/main/bolt/lib/Profile/YAMLProfileWriter.cpp#L162-L176

Test Plan: NFCI


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

1 Files Affected:

  • (modified) bolt/lib/Profile/DataAggregator.cpp (+9-5)
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 10d745cc69824b..4aeeb1daab1b94 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -2427,11 +2427,15 @@ std::error_code DataAggregator::writeBATYAML(BinaryContext &BC,
           }
         }
       }
-      // Drop blocks without a hash, won't be useful for stale matching.
-      llvm::erase_if(YamlBF.Blocks,
-                     [](const yaml::bolt::BinaryBasicBlockProfile &YamlBB) {
-                       return YamlBB.Hash == (yaml::Hex64)0;
-                     });
+      // Skip printing if there's no profile data
+      llvm::erase_if(
+          YamlBF.Blocks, [](const yaml::bolt::BinaryBasicBlockProfile &YamlBB) {
+            auto HasCount = [](const auto &SI) { return SI.Count; };
+            bool HasAnyCount = YamlBB.ExecCount ||
+                               llvm::any_of(YamlBB.Successors, HasCount) ||
+                               llvm::any_of(YamlBB.CallSites, HasCount);
+            return !HasAnyCount;
+          });
       BP.Functions.emplace_back(YamlBF);
     }
   }

Copy link
Contributor

@rafaelauler rafaelauler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

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.bolt-drop-blocks-without-profile-in-bat-yaml to main September 11, 2024 23:34
@aaupov aaupov changed the base branch from main to users/aaupov/spr/main.bolt-drop-blocks-without-profile-in-bat-yaml September 11, 2024 23:35
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.bolt-drop-blocks-without-profile-in-bat-yaml to main September 11, 2024 23:36
@aaupov aaupov merged commit ccc7a07 into main Sep 11, 2024
6 of 8 checks passed
@aaupov aaupov deleted the users/aaupov/spr/bolt-drop-blocks-without-profile-in-bat-yaml branch September 11, 2024 23:36
aaupov added a commit that referenced this pull request Sep 13, 2024
Add probe inline tree information to YAML profile, at function level:
- function GUID,
- checksum,
- parent node id,
- call site in the parent.

This information is used for pseudo probe block matching (#99891).

The encoding adds/changes probe information in multiple levels of
YAML profile:
- BinaryProfile: add pseudo_probe_desc with GUIDs and Hashes, which
  permits deduplication of data:
  - many GUIDs are duplicate as the same callee is commonly inlined
    into multiple callers,
  - hashes are also very repetitive, especially for functions with
    low block counts.
- FunctionProfile: add inline tree (see above). Top-level function
  is included as root of function inline tree, which makes guid and
  pseudo_probe_desc_hash fields redundant.
- BlockProfile: densely-encoded block probe information:
  - probes reference their containing inline tree node,
  - separate lists for block, call, indirect call probes,
  - block probe encoding is specialized: ids are encoded as bitset
    in uint64_t. If only block probe with id=1 is present, it's
    encoded as implicit entry (id=0, omitted).
  - inline tree nodes with identical probes share probe description
    where node indices are combined into a list.

On top of #107970, profile with new probe encoding has the following
characteristics (profile for a large binary):

- Profile without probe information: 33MB, 3.8MB compressed (baseline).
- Profile with inline tree information: 92MB, 14MB compressed.

Profile processing time (YAML parsing, inference, attaching steps):
- profile without pseudo probes: 5s,
- profile with pseudo probes, without pseudo probe matching: 11s,
- with pseudo probe matching: 12.5s.

Test Plan: updated pseudoprobe-decoding-inline.test

Reviewers: wlei-llvm, ayermolo, rafaelauler, dcci, maksfb

Reviewed By: wlei-llvm, rafaelauler

Pull Request: #107137
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.

3 participants