[llvm] [BOLT][NFC] Rename isUnsupportedBranch to isReversibleBranch (PR #92447)

Nathan Sidwell via llvm-commits llvm-commits at lists.llvm.org
Thu May 16 12:57:28 PDT 2024


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

`isUnsupportedBranch` is not a very informative name, and doesn't match its corresponding `reverseBranchCondition`, as I noted in PR #92018.  Here's a renaming to a more mnemonic name.

>From 542f461a8e0dc12da1a06ef57d83043090637d02 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan at acm.org>
Date: Thu, 16 May 2024 14:57:09 -0400
Subject: [PATCH] [BOLT][NFC] Rename isUnsupportedBranch to isReversibleBranch

---
 bolt/include/bolt/Core/MCPlusBuilder.h   |  4 ++--
 bolt/lib/Core/BinaryFunction.cpp         |  4 ++--
 bolt/lib/Passes/Instrumentation.cpp      |  2 +-
 bolt/lib/Target/X86/X86MCPlusBuilder.cpp | 10 +++++-----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h
index f7614cf9ac977..f7cf538bd0e86 100644
--- a/bolt/include/bolt/Core/MCPlusBuilder.h
+++ b/bolt/include/bolt/Core/MCPlusBuilder.h
@@ -438,8 +438,8 @@ class MCPlusBuilder {
     return false;
   }
 
-  /// Check whether we support inverting this branch
-  virtual bool isUnsupportedBranch(const MCInst &Inst) const { return false; }
+  /// Check whether this conditional branch can be reversed
+  virtual bool isReversibleBranch(const MCInst &Inst) const { return true; }
 
   /// Return true of the instruction is of pseudo kind.
   virtual bool isPseudo(const MCInst &Inst) const {
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index de34421ebeb08..a9c588fc6e060 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1284,7 +1284,7 @@ Error BinaryFunction::disassemble() {
         const bool IsCondBranch = MIB->isConditionalBranch(Instruction);
         MCSymbol *TargetSymbol = nullptr;
 
-        if (BC.MIB->isUnsupportedBranch(Instruction)) {
+        if (!BC.MIB->isReversibleBranch(Instruction)) {
           setIgnored();
           if (BinaryFunction *TargetFunc =
                   BC.getBinaryFunctionContainingAddress(TargetAddress))
@@ -3384,7 +3384,7 @@ void BinaryFunction::fixBranches() {
 
       // Reverse branch condition and swap successors.
       auto swapSuccessors = [&]() {
-        if (MIB->isUnsupportedBranch(*CondBranch)) {
+        if (!MIB->isReversibleBranch(*CondBranch)) {
           if (opts::Verbosity) {
             BC.outs() << "BOLT-INFO: unable to swap successors in " << *this
                       << '\n';
diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp
index 68acff7e6a867..14f506f9ca968 100644
--- a/bolt/lib/Passes/Instrumentation.cpp
+++ b/bolt/lib/Passes/Instrumentation.cpp
@@ -480,7 +480,7 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function,
       else if (BC.MIB->isUnconditionalBranch(Inst))
         HasUnconditionalBranch = true;
       else if ((!BC.MIB->isCall(Inst) && !BC.MIB->isConditionalBranch(Inst)) ||
-               BC.MIB->isUnsupportedBranch(Inst))
+               !BC.MIB->isReversibleBranch(Inst))
         continue;
 
       const uint32_t FromOffset = *BC.MIB->getOffset(Inst);
diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
index 8b1894953f375..8fdacffcb147b 100644
--- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
+++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp
@@ -328,19 +328,19 @@ class X86MCPlusBuilder : public MCPlusBuilder {
     return false;
   }
 
-  bool isUnsupportedBranch(const MCInst &Inst) const override {
+  bool isReversibleBranch(const MCInst &Inst) const override {
     if (isDynamicBranch(Inst))
-      return true;
+      return false;
 
     switch (Inst.getOpcode()) {
     default:
-      return false;
+      return true;
     case X86::LOOP:
     case X86::LOOPE:
     case X86::LOOPNE:
     case X86::JECXZ:
     case X86::JRCXZ:
-      return true;
+      return false;
     }
   }
 
@@ -1874,7 +1874,7 @@ class X86MCPlusBuilder : public MCPlusBuilder {
       }
 
       // Handle conditional branches and ignore indirect branches
-      if (!isUnsupportedBranch(*I) && getCondCode(*I) == X86::COND_INVALID) {
+      if (isReversibleBranch(*I) && getCondCode(*I) == X86::COND_INVALID) {
         // Indirect branch
         return false;
       }



More information about the llvm-commits mailing list