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

via llvm-commits llvm-commits at lists.llvm.org
Fri May 17 03:56:01 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Nathan Sidwell (urnathan)

<details>
<summary>Changes</summary>

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

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


4 Files Affected:

- (modified) bolt/include/bolt/Core/MCPlusBuilder.h (+2-2) 
- (modified) bolt/lib/Core/BinaryFunction.cpp (+2-2) 
- (modified) bolt/lib/Passes/Instrumentation.cpp (+1-1) 
- (modified) bolt/lib/Target/X86/X86MCPlusBuilder.cpp (+5-5) 


``````````diff
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;
       }

``````````

</details>


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


More information about the llvm-commits mailing list