[PATCH] D129204: [X86][NFCI] Remove target-specific branch optimisation that's handled in BranchFolding

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added a reviewer: craig.topper.
Herald added subscribers: jsji, wingo, pmatos, luke957, StephenFan, luismarques, sameer.abuasal, pengfei, s.egerton, PkmX, simoncook, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added a project: LLVM.

I was looking at other target's implementations of analyzeBranch to see if there's anything we're neglecting to handle for RISC-V. It looks like this optimisation in analyzeBranch is handled in OptimizeBlock in BranchFolding, so it's not clear there's any advantage in repeating in in X86's analyzeBranch. Certainly, no tests fail upon removing it.

Perhaps there's a benefit of handling it here that I'm missing?


https://reviews.llvm.org/D129204

Files:
  llvm/lib/Target/X86/X86InstrInfo.cpp


Index: llvm/lib/Target/X86/X86InstrInfo.cpp
===================================================================
--- llvm/lib/Target/X86/X86InstrInfo.cpp
+++ llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -3074,44 +3074,6 @@
 
     // Working from the bottom, handle the first conditional branch.
     if (Cond.empty()) {
-      MachineBasicBlock *TargetBB = I->getOperand(0).getMBB();
-      if (AllowModify && UnCondBrIter != MBB.end() &&
-          MBB.isLayoutSuccessor(TargetBB)) {
-        // If we can modify the code and it ends in something like:
-        //
-        //     jCC L1
-        //     jmp L2
-        //   L1:
-        //     ...
-        //   L2:
-        //
-        // Then we can change this to:
-        //
-        //     jnCC L2
-        //   L1:
-        //     ...
-        //   L2:
-        //
-        // Which is a bit more efficient.
-        // We conditionally jump to the fall-through block.
-        BranchCode = GetOppositeBranchCondition(BranchCode);
-        MachineBasicBlock::iterator OldInst = I;
-
-        BuildMI(MBB, UnCondBrIter, MBB.findDebugLoc(I), get(X86::JCC_1))
-          .addMBB(UnCondBrIter->getOperand(0).getMBB())
-          .addImm(BranchCode);
-        BuildMI(MBB, UnCondBrIter, MBB.findDebugLoc(I), get(X86::JMP_1))
-          .addMBB(TargetBB);
-
-        OldInst->eraseFromParent();
-        UnCondBrIter->eraseFromParent();
-
-        // Restart the analysis.
-        UnCondBrIter = MBB.end();
-        I = MBB.end();
-        continue;
-      }
-
       FBB = TBB;
       TBB = I->getOperand(0).getMBB();
       Cond.push_back(MachineOperand::CreateImm(BranchCode));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129204.442567.patch
Type: text/x-patch
Size: 1629 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220706/f13665f0/attachment.bin>