[llvm-branch-commits] [BOLT] Support negative hex input (-1) for BR_ONLY in pre-aggregated profiles (PR #192391)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

<details>
<summary>Changes</summary>

Handle signed values in parseHexField by falling back to int64_t parsing
when uint64_t fails. This allows pre-aggregated profile tools to use -1
for BR_ONLY, -2 for FT_EXTERNAL_ORIGIN, -3 for FT_EXTERNAL_RETURN.

Guard the external address reset loop in parseAggregatedLBREntry to
preserve sentinel values (offsets >= FT_EXTERNAL_RETURN).

Add tests for -1/-2/-3 in parseHexField and T entries with -1,
ffffffffffffffff, and buildid:-1 as BR_ONLY.


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


3 Files Affected:

- (modified) bolt/lib/Profile/DataAggregator.cpp (+4-2) 
- (modified) bolt/lib/Profile/DataReader.cpp (+8-3) 
- (modified) bolt/unittests/Profile/DataAggregator.cpp (+40) 


``````````diff
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 08ae24bfd411c..344682f9ae2f4 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -1455,9 +1455,11 @@ std::error_code DataAggregator::parseAggregatedLBREntry() {
     return std::error_code();
   }
 
-  // Reset external addresses.
+  // Reset external addresses, but preserve sentinel values (BR_ONLY,
+  // FT_EXTERNAL_ORIGIN, FT_EXTERNAL_RETURN).
   for (std::optional<Location> &Loc : Addr)
-    if (Loc && Loc->Name != FilterBuildID)
+    if (Loc && Loc->Offset < Trace::FT_EXTERNAL_RETURN &&
+        Loc->Name != FilterBuildID)
       Loc->Offset = Trace::EXTERNAL;
 
   const uint64_t FromOffset = Addr[0]->Offset;
diff --git a/bolt/lib/Profile/DataReader.cpp b/bolt/lib/Profile/DataReader.cpp
index d3bea76daa5b9..38e32d12028d3 100644
--- a/bolt/lib/Profile/DataReader.cpp
+++ b/bolt/lib/Profile/DataReader.cpp
@@ -883,9 +883,14 @@ ErrorOr<uint64_t> DataReader::parseHexField(char EndChar, bool EndNl) {
   StringRef NumStr = NumStrRes.get();
   uint64_t Num;
   if (NumStr.getAsInteger(16, Num)) {
-    reportError("expected hexadecimal number");
-    Diag << "Found: " << NumStr << "\n";
-    return make_error_code(llvm::errc::io_error);
+    // Accept signed input (e.g. -1) to support sentinel values like BR_ONLY.
+    int64_t SignedNum;
+    if (NumStr.getAsInteger(16, SignedNum)) {
+      reportError("expected hexadecimal number");
+      Diag << "Found: " << NumStr << "\n";
+      return make_error_code(llvm::errc::io_error);
+    }
+    return static_cast<uint64_t>(SignedNum);
   }
   return Num;
 }
diff --git a/bolt/unittests/Profile/DataAggregator.cpp b/bolt/unittests/Profile/DataAggregator.cpp
index 8e23b17d9f26c..6f417df1e17b6 100644
--- a/bolt/unittests/Profile/DataAggregator.cpp
+++ b/bolt/unittests/Profile/DataAggregator.cpp
@@ -131,6 +131,21 @@ TEST_F(PreAggregatedTestHelper, parseHexField) {
   ASSERT_TRUE(!!Res);
   EXPECT_EQ(*Res, Trace::BR_ONLY);
 
+  // -1 → UINT64_MAX (BR_ONLY / FT_ONLY).
+  Res = parseHex("-1\n");
+  ASSERT_TRUE(!!Res);
+  EXPECT_EQ(*Res, Trace::BR_ONLY);
+
+  // -2 → UINT64_MAX - 1 (FT_EXTERNAL_ORIGIN).
+  Res = parseHex("-2\n");
+  ASSERT_TRUE(!!Res);
+  EXPECT_EQ(*Res, Trace::FT_EXTERNAL_ORIGIN);
+
+  // -3 → UINT64_MAX - 2 (FT_EXTERNAL_RETURN).
+  Res = parseHex("-3\n");
+  ASSERT_TRUE(!!Res);
+  EXPECT_EQ(*Res, Trace::FT_EXTERNAL_RETURN);
+
   Res = parseHex("0\n");
   ASSERT_TRUE(!!Res);
   EXPECT_EQ(*Res, 0ULL);
@@ -234,6 +249,31 @@ TEST_F(PreAggregatedX86TestHelper, ReturnEntry) {
   EXPECT_EQ(Traces[0].second.TakenCount, 4u);
 }
 
+TEST_F(PreAggregatedX86TestHelper, TraceWithNeg1AsBROnly) {
+  // T entry with -1 as fall-through target: parsed as BR_ONLY.
+  std::vector<std::pair<Trace, TakenBranchInfo>> Traces;
+  parseAndCollectTraces("T 4b196f 4b19e0 -1 2\n", Traces);
+  ASSERT_EQ(Traces.size(), 1u);
+  EXPECT_EQ(Traces[0].first.Branch, 0x4b196fULL);
+  EXPECT_EQ(Traces[0].first.From, 0x4b19e0ULL);
+  EXPECT_EQ(Traces[0].first.To, Trace::BR_ONLY);
+  EXPECT_EQ(Traces[0].second.TakenCount, 2u);
+}
+
+TEST_F(PreAggregatedX86TestHelper, TraceWithFFFAsBROnly) {
+  std::vector<std::pair<Trace, TakenBranchInfo>> Traces;
+  parseAndCollectTraces("T 4b196f 4b19e0 ffffffffffffffff 2\n", Traces);
+  ASSERT_EQ(Traces.size(), 1u);
+  EXPECT_EQ(Traces[0].first.To, Trace::BR_ONLY);
+}
+
+TEST_F(PreAggregatedX86TestHelper, TraceWithBuildIdNeg1) {
+  std::vector<std::pair<Trace, TakenBranchInfo>> Traces;
+  parseAndCollectTraces("T 4b196f 4b19e0 deadbeef:-1 2\n", Traces);
+  ASSERT_EQ(Traces.size(), 1u);
+  EXPECT_EQ(Traces[0].first.To, Trace::BR_ONLY);
+}
+
 TEST_F(PreAggregatedX86TestHelper, MultipleEntries) {
   std::vector<std::pair<Trace, TakenBranchInfo>> Traces;
   parseAndCollectTraces("B 100 200 1 0\n"

``````````

</details>


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