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

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Nathan Sidwell (urnathan)

<details>
<summary>Changes</summary>

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.

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


1 Files Affected:

- (modified) bolt/lib/Core/BinaryFunction.cpp (-7) 


``````````diff
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.

``````````

</details>


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