[llvm] [BOLT] Do not reject irreversible branches during disassembly (PR #95572)

Nathan Sidwell via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 14 10:08:53 PDT 2024


https://github.com/urnathan created https://github.com/llvm/llvm-project/pull/95572

Now that `isUnsupportedBranch` is renamed (and inverted) to `isReversibleBranch`, this use in `BinaryFunction::disassemble` sticks out like a sore thumb. My guess is that at one point one needed to reject unintelligible branches early, and that morphed into the use for detecting uninvertible branches.

Removing this check results in no new test failures.

The use of `isReversibleBranch` here raises questions about what it should return for instructions that are not conditional branches -- given the other uses of this function I had been presuming 'don't care'.

If some branches should be rejected at this point, a new MCPlusBuilder hook is most likely needed -- this cannot be the correct predicate to use (after all, it would render all the other uses moot.

>From 8e3a6edbdfdcc0ed43d2fe3ca40c1dfc6cd474cb Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan at acm.org>
Date: Fri, 14 Jun 2024 13:00:20 -0400
Subject: [PATCH] [BOLT] Do not reject irreversible branches during disassembly

Now that `isUnsupportedBranch` is renamed (and inverted) to
`isReversibleBranch`, this use in `BinaryFunction::disassemble` sticks
out like a sore thumb. My guess is that at one point one needed to
reject unintelligible branches early, and that morphed into the use
for detecting uninvertible branches.

Removing this check results in no new test failures.

The use of `isReversibleBranch` here raises questions about what it
should return for instructions that are not conditional branches --
given the other uses of this function I had been presuming 'don't
care'.

If some branches should be rejected at this point, a new MCPlusBuilder
hook is most likely needed -- this cannot be the correct predicate to
use (after all, it would render all the other uses moot.
---
 bolt/lib/Core/BinaryFunction.cpp | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index d13e28999a05c..a8b1f69166d21 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1288,13 +1288,6 @@ Error BinaryFunction::disassemble() {
         const bool IsCondBranch = MIB->isConditionalBranch(Instruction);
         MCSymbol *TargetSymbol = nullptr;
 
-        if (!BC.MIB->isReversibleBranch(Instruction)) {
-          setIgnored();
-          if (BinaryFunction *TargetFunc =
-                  BC.getBinaryFunctionContainingAddress(TargetAddress))
-            TargetFunc->setIgnored();
-        }
-
         if (IsCall && containsAddress(TargetAddress)) {
           if (TargetAddress == getAddress()) {
             // Recursive call.



More information about the llvm-commits mailing list