[all-commits] [llvm/llvm-project] bce889: [BOLT] Align BranchInfo and FuncBranchData in Data...

[Amir Ayupov via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: bce889c8df419d639beb0d387409d1ecbefdf579
      https://github.com/llvm/llvm-project/commit/bce889c8df419d639beb0d387409d1ecbefdf579
  Author: Amir Ayupov <aaupov at fb.com>
  Date:   2023-05-30 (Tue, 30 May 2023)

  Changed paths:
    M bolt/include/bolt/Profile/DataAggregator.h
    M bolt/lib/Profile/DataAggregator.cpp

  Log Message:
  -----------
  [BOLT] Align BranchInfo and FuncBranchData in DataAggregator::recordTrace

`DataAggregator::recordTrace` serves two purposes:
  - Attaching LBR fallthrough ("trace") information to CFG (`getBranchInfo`),
    which eventually gets emitted as YAML profile.
  - Populating vector of offsets that gets added to `FuncBranchData`, which
    eventually gets emitted as fdata profile.

`recordTrace` is invoked from `getFallthroughsInTrace` which checks its return
status and passes on the collected vector of offsets to `doTrace`.

However, if a malformed trace is passed to `recordTrace` it might partially
attach the profile to CFG and exit with false, not propagating the vector of
offsets to `doTrace`. This leads to a difference between fdata and yaml profile
collected from the same binary and the same perf file.

(Skylake LBR errata might produce such malformed traces where the last entry
is duplicated, resulting in invalid fallthrough path between the last two
entries).

There are two ways to handle this mismatch: conservative (aligned with fdata),
or aggressive (aligned with yaml). Conservative approach would discard the
trace entirely, buffering the CFG updates until all fallthroughs are confirmed.
Aggressive approach would apply CFG updates and return the matching
fallthroughs in the vector even if the trace is invalid (doesn't correspond to
a valid fallthrough path). I chose to go with the former (conservative/fdata)
approach which produces more accurate profile.

We can't rely on pre-filtering such traces early (in LBR sample processing) as
DataAggregator is used for both perf samples and pre-aggregated perf information
which loses branch stack information.

Test Plan: https://github.com/rafaelauler/bolt-tests/pull/22

Reviewed By: #bolt, rafauler

Differential Revision: https://reviews.llvm.org/D151614