[llvm-branch-commits] [llvm] [BOLT] Ignore returns in DataAggregator (PR #90807)

[Amir Ayupov via llvm-branch-commits]

https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/90807

>From acf58ceb37d2aa917e8d84d243faadc58f5f3a7d Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Mon, 6 May 2024 13:35:04 -0700
Subject: [PATCH 1/3] Simplify IsReturn check

Created using spr 1.3.4
---
 bolt/lib/Profile/DataAggregator.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index e4a7324c38175..d02e4499014ed 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -778,13 +778,13 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
     if (BinaryFunction *Func = getBinaryFunctionContainingAddress(Addr)) {
       Addr -= Func->getAddress();
       if (IsFrom) {
-        if (Func->hasInstructions()) {
-          if (MCInst *Inst = Func->getInstructionAtOffset(Addr))
-            IsReturn = BC->MIB->isReturn(*Inst);
-        } else if (std::optional<MCInst> Inst =
-                Func->disassembleInstructionAtOffset(Addr)) {
-          IsReturn = BC->MIB->isReturn(*Inst);
-        }
+        auto checkReturn = [&](auto MaybeInst) {
+          IsReturn = MaybeInst && BC->MIB->isReturn(*MaybeInst);
+        };
+        if (Func->hasInstructions())
+          checkReturn(Func->getInstructionAtOffset(Addr));
+        else
+          checkReturn(Func->disassembleInstructionAtOffset(Addr));
       }
 
       if (BAT)

>From 22052e461e5671f376fe2dcb733446b0a63e956d Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Tue, 7 May 2024 18:30:48 -0700
Subject: [PATCH 2/3] drop const from disassembleInstructionAtOffset

Created using spr 1.3.4
---
 bolt/include/bolt/Core/BinaryFunction.h | 3 +--
 bolt/lib/Core/BinaryFunction.cpp        | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index b21312f92c485..3c641581e247a 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -930,8 +930,7 @@ class BinaryFunction {
     return const_cast<BinaryFunction *>(this)->getInstructionAtOffset(Offset);
   }
 
-  const std::optional<MCInst>
-  disassembleInstructionAtOffset(uint64_t Offset) const;
+  std::optional<MCInst> disassembleInstructionAtOffset(uint64_t Offset) const;
 
   /// Return offset for the first instruction. If there is data at the
   /// beginning of a function then offset of the first instruction could
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 5f3c0cb1ad754..fb81fc3f2ba7b 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1167,7 +1167,7 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst &Instruction,
   }
 }
 
-const std::optional<MCInst>
+std::optional<MCInst>
 BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
   assert(CurrentState == State::Empty);
   assert(Offset < MaxSize && "invalid offset");

>From 63725510bf85c9e3862800830f5881099ab4b21f Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 8 May 2024 11:59:59 -0700
Subject: [PATCH 3/3] Assert messages

Created using spr 1.3.4
---
 bolt/lib/Core/BinaryFunction.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index fb81fc3f2ba7b..4721a247ee2e2 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1169,10 +1169,10 @@ void BinaryFunction::handleAArch64IndirectCall(MCInst &Instruction,
 
 std::optional<MCInst>
 BinaryFunction::disassembleInstructionAtOffset(uint64_t Offset) const {
-  assert(CurrentState == State::Empty);
-  assert(Offset < MaxSize && "invalid offset");
+  assert(CurrentState == State::Empty && "Function should not be disassembled");
+  assert(Offset < MaxSize && "Invalid offset");
   ErrorOr<ArrayRef<unsigned char>> FunctionData = getData();
-  assert(FunctionData && "cannot get function as data");
+  assert(FunctionData && "Cannot get function as data");
   MCInst Instr;
   uint64_t InstrSize = 0;
   const uint64_t InstrAddress = getAddress() + Offset;