[llvm-branch-commits] [BOLT][BAT] Fix translate for branches added by BOLT (PR #90811)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

<details>
<summary>Changes</summary>

Map branches that are not present in BAT to the containing basic block.
The normal behavior is to map it to the preceding translation entry,
which may or may not be a basic block, and this causes profile staleness
issues.

This diff fulfills the intent of the comment added in 21f4303bfd01d:
```
// Branch source addresses are translated to the first instruction of the
// source BB to avoid accounting for modifications BOLT may have made in the
// BB regarding deletion/addition of instructions.
```

Test Plan: Updated bolt-address-translation-yaml.test


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


2 Files Affected:

- (modified) bolt/lib/Profile/BoltAddressTranslation.cpp (+22-5) 
- (modified) bolt/test/X86/bolt-address-translation-yaml.test (+6-1) 


``````````diff
diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp
index 7cfb9c132c2c68..ce477d642c7627 100644
--- a/bolt/lib/Profile/BoltAddressTranslation.cpp
+++ b/bolt/lib/Profile/BoltAddressTranslation.cpp
@@ -478,6 +478,13 @@ uint64_t BoltAddressTranslation::translate(uint64_t FuncAddress,
     return Offset;
 
   const MapTy &Map = Iter->second;
+  if (IsBranchSrc) {
+    // Try exact lookup first
+    auto KeyVal = Map.find(Offset);
+    if (KeyVal != Map.end())
+      if (KeyVal->second & BRANCHENTRY)
+        return KeyVal->second >> 1;
+  }
   auto KeyVal = Map.upper_bound(Offset);
   if (KeyVal == Map.begin())
     return Offset;
@@ -485,11 +492,21 @@ uint64_t BoltAddressTranslation::translate(uint64_t FuncAddress,
   --KeyVal;
 
   const uint32_t Val = KeyVal->second >> 1; // dropping BRANCHENTRY bit
-  // Branch source addresses are translated to the first instruction of the
-  // source BB to avoid accounting for modifications BOLT may have made in the
-  // BB regarding deletion/addition of instructions.
-  if (IsBranchSrc)
-    return Val;
+  if (IsBranchSrc) {
+    // Branch source addresses are translated to the first instruction of the
+    // source BB to avoid accounting for modifications BOLT may have made in the
+    // BB regarding deletion/addition of instructions.
+    const uint64_t ParentAddress = fetchParentAddress(FuncAddress);
+    const BBHashMapTy &BBHashMap =
+        getBBHashMap(ParentAddress ? ParentAddress : FuncAddress);
+    auto BBIt = BBHashMap.upper_bound(Val);
+    if (BBIt == BBHashMap.begin())
+      return Offset;
+
+    --BBIt;
+
+    return BBIt->first;
+  }
   return Offset - KeyVal->first + Val;
 }
 
diff --git a/bolt/test/X86/bolt-address-translation-yaml.test b/bolt/test/X86/bolt-address-translation-yaml.test
index b3d8a88394503c..59c97317b2f83f 100644
--- a/bolt/test/X86/bolt-address-translation-yaml.test
+++ b/bolt/test/X86/bolt-address-translation-yaml.test
@@ -9,7 +9,8 @@ RUN: perf2bolt %t.out --pa -p %p/Inputs/blarge_new_bat.preagg.txt -w %t.yaml -o
 RUN:   2>&1 | FileCheck --check-prefix READ-BAT-CHECK %s
 RUN: FileCheck --input-file %t.yaml --check-prefix YAML-BAT-CHECK %s
 # Check that YAML converted from fdata matches YAML created directly with BAT.
-RUN: llvm-bolt %t.exe -data %t.fdata -w %t.yaml-fdata -o /dev/null
+RUN: llvm-bolt %t.exe -data %t.fdata -w %t.yaml-fdata -o /dev/null \
+RUN:   | FileCheck --check-prefix CHECK-BOLT-FDATA %s
 RUN: FileCheck --input-file %t.yaml-fdata --check-prefix YAML-BAT-CHECK %s
 
 # Test resulting YAML profile with the original binary (no-stale mode)
@@ -68,3 +69,7 @@ YAML-BAT-CHECK-NEXT:       succ:  {{.*}} { bid: 2, cnt: [[#]]
 CHECK-BOLT-YAML:      pre-processing profile using YAML profile reader
 CHECK-BOLT-YAML-NEXT: 5 out of 16 functions in the binary (31.2%) have non-empty execution profile
 CHECK-BOLT-YAML-NOT: invalid (possibly stale) profile
+CHECK-BOLT-FDATA:      pre-processing profile using branch profile reader
+CHECK-BOLT-FDATA-NEXT: profile collection done on a binary already processed by BOLT
+CHECK-BOLT-FDATA-NEXT: 5 out of 16 functions in the binary (31.2%) have non-empty execution profile
+CHECK-BOLT-FDATA-NOT: invalid (possibly stale) profile

``````````

</details>


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