Temporarily revert "Update branch coalescing to be a PowerPC specific pass"

From comments and code review it wasn't intended to be enabled by default yet.

This reverts commit r311588.

llvm-svn: 312214

llvm/include/llvm/CodeGen/Passes.h

@@ -409,6 +409,9 @@ namespace llvm {
   /// This pass frees the memory occupied by the MachineFunction.
   FunctionPass *createFreeMachineFunctionPass();
 
+  /// This pass combine basic blocks guarded by the same branch.
+  extern char &BranchCoalescingID;
+
   /// This pass performs outlining on machine instructions directly before
   /// printing assembly.
   ModulePass *createMachineOutlinerPass();

llvm/include/llvm/InitializePasses.h

@@ -76,6 +76,7 @@ void initializeBasicAAWrapperPassPass(PassRegistry&);
 void initializeBlockExtractorPassPass(PassRegistry&);
 void initializeBlockFrequencyInfoWrapperPassPass(PassRegistry&);
 void initializeBoundsCheckingPass(PassRegistry&);
+void initializeBranchCoalescingPass(PassRegistry&);
 void initializeBranchFolderPassPass(PassRegistry&);
 void initializeBranchProbabilityInfoWrapperPassPass(PassRegistry&);
 void initializeBranchRelaxationPass(PassRegistry&);

llvm/lib/CodeGen/BranchCoalescing.cpp

@@ -13,7 +13,6 @@
 ///
 //===----------------------------------------------------------------------===//
 
-#include "PPC.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/CodeGen/MachineDominators.h"
@@ -28,18 +27,18 @@
 
 using namespace llvm;
 
-#define DEBUG_TYPE "ppc-branch-coalescing"
+#define DEBUG_TYPE "branch-coalescing"
+
+static cl::opt<cl::boolOrDefault>
+    EnableBranchCoalescing("enable-branch-coalesce", cl::Hidden,
+                           cl::desc("enable coalescing of duplicate branches"));
 
 STATISTIC(NumBlocksCoalesced, "Number of blocks coalesced");
 STATISTIC(NumPHINotMoved, "Number of PHI Nodes that cannot be merged");
 STATISTIC(NumBlocksNotCoalesced, "Number of blocks not coalesced");
 
-namespace llvm {
-    void initializePPCBranchCoalescingPass(PassRegistry&);
-}
-
 //===----------------------------------------------------------------------===//
-//                               PPCBranchCoalescing
+//                               BranchCoalescing
 //===----------------------------------------------------------------------===//
 ///
 /// Improve scheduling by coalescing branches that depend on the same condition.
@@ -47,17 +46,13 @@ namespace llvm {
 /// and attempts to merge the blocks together. Such opportunities arise from
 /// the expansion of select statements in the IR.
 ///
-/// This pass does not handle implicit operands on branch statements. In order
+/// For example, consider the following LLVM IR:
-/// to run on targets that use implicit operands, changes need to be made in the
-/// canCoalesceBranch and canMerge methods.
 ///
-/// Example: the following LLVM IR
+/// %test = icmp eq i32 %x 0
+/// %tmp1 = select i1 %test, double %a, double 2.000000e-03
+/// %tmp2 = select i1 %test, double %b, double 5.000000e-03
 ///
-///     %test = icmp eq i32 %x 0
+/// This IR expands to the following machine code on PowerPC:
-///     %tmp1 = select i1 %test, double %a, double 2.000000e-03
-///     %tmp2 = select i1 %test, double %b, double 5.000000e-03
-///
-/// expands to the following machine code:
 ///
 /// BB#0: derived from LLVM BB %entry
 ///    Live Ins: %F1 %F3 %X6
@@ -137,7 +132,7 @@ namespace llvm {
 
 namespace {
 
-class PPCBranchCoalescing : public MachineFunctionPass {
+class BranchCoalescing : public MachineFunctionPass {
   struct CoalescingCandidateInfo {
     MachineBasicBlock *BranchBlock;       // Block containing the branch
     MachineBasicBlock *BranchTargetBlock; // Block branched to
@@ -162,11 +157,15 @@ class PPCBranchCoalescing : public MachineFunctionPass {
   bool validateCandidates(CoalescingCandidateInfo &SourceRegion,
                           CoalescingCandidateInfo &TargetRegion) const;
 
+  static bool isBranchCoalescingEnabled() {
+    return EnableBranchCoalescing == cl::BOU_TRUE;
+  }
+
 public:
   static char ID;
 
-  PPCBranchCoalescing() : MachineFunctionPass(ID) {
+  BranchCoalescing() : MachineFunctionPass(ID) {
-    initializePPCBranchCoalescingPass(*PassRegistry::getPassRegistry());
+    initializeBranchCoalescingPass(*PassRegistry::getPassRegistry());
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
@@ -191,25 +190,21 @@ public:
 };
 } // End anonymous namespace.
 
-char PPCBranchCoalescing::ID = 0;
+char BranchCoalescing::ID = 0;
-/// createPPCBranchCoalescingPass - returns an instance of the Branch Coalescing
+char &llvm::BranchCoalescingID = BranchCoalescing::ID;
-/// Pass
-FunctionPass *llvm::createPPCBranchCoalescingPass() {
-  return new PPCBranchCoalescing();
-}
 
-INITIALIZE_PASS_BEGIN(PPCBranchCoalescing, DEBUG_TYPE,
+INITIALIZE_PASS_BEGIN(BranchCoalescing, DEBUG_TYPE,
                       "Branch Coalescing", false, false)
 INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
 INITIALIZE_PASS_DEPENDENCY(MachinePostDominatorTree)
-INITIALIZE_PASS_END(PPCBranchCoalescing, DEBUG_TYPE, "Branch Coalescing",
+INITIALIZE_PASS_END(BranchCoalescing, DEBUG_TYPE, "Branch Coalescing",
                     false, false)
 
-PPCBranchCoalescing::CoalescingCandidateInfo::CoalescingCandidateInfo()
+BranchCoalescing::CoalescingCandidateInfo::CoalescingCandidateInfo()
     : BranchBlock(nullptr), BranchTargetBlock(nullptr),
       FallThroughBlock(nullptr), MustMoveDown(false), MustMoveUp(false) {}
 
-void PPCBranchCoalescing::CoalescingCandidateInfo::clear() {
+void BranchCoalescing::CoalescingCandidateInfo::clear() {
   BranchBlock = nullptr;
   BranchTargetBlock = nullptr;
   FallThroughBlock = nullptr;
@@ -218,7 +213,7 @@ void PPCBranchCoalescing::CoalescingCandidateInfo::clear() {
   MustMoveUp = false;
 }
 
-void PPCBranchCoalescing::initialize(MachineFunction &MF) {
+void BranchCoalescing::initialize(MachineFunction &MF) {
   MDT = &getAnalysis<MachineDominatorTree>();
   MPDT = &getAnalysis<MachinePostDominatorTree>();
   TII = MF.getSubtarget().getInstrInfo();
@@ -235,7 +230,7 @@ void PPCBranchCoalescing::initialize(MachineFunction &MF) {
 ///\param[in,out] Cand The coalescing candidate to analyze
 ///\return true if and only if the branch can be coalesced, false otherwise
 ///
-bool PPCBranchCoalescing::canCoalesceBranch(CoalescingCandidateInfo &Cand) {
+bool BranchCoalescing::canCoalesceBranch(CoalescingCandidateInfo &Cand) {
   DEBUG(dbgs() << "Determine if branch block " << Cand.BranchBlock->getNumber()
                << " can be coalesced:");
   MachineBasicBlock *FalseMBB = nullptr;
@@ -251,19 +246,6 @@ bool PPCBranchCoalescing::canCoalesceBranch(CoalescingCandidateInfo &Cand) {
     if (!I.isBranch())
       continue;
 
-    // The analyzeBranch method does not include any implicit operands.
-    // This is not an issue on PPC but must be handled on other targets.
-    // For this pass to be made target-independent, the analyzeBranch API
-    // need to be updated to support implicit operands and there would
-    // need to be a way to verify that any implicit operands would not be
-    // clobbered by merging blocks.  This would include identifying the
-    // implicit operands as well as the basic block they are defined in.
-    // This could be done by changing the analyzeBranch API to have it also
-    // record and return the implicit operands and the blocks where they are
-    // defined. Alternatively, the BranchCoalescing code would need to be
-    // extended to identify the implicit operands.  The analysis in canMerge
-    // must then be extended to prove that none of the implicit operands are
-    // changed in the blocks that are combined during coalescing.
     if (I.getNumOperands() != I.getNumExplicitOperands()) {
       DEBUG(dbgs() << "Terminator contains implicit operands - skip : " << I
                    << "\n");
@@ -327,7 +309,7 @@ bool PPCBranchCoalescing::canCoalesceBranch(CoalescingCandidateInfo &Cand) {
 /// \param[in] OpList2 operand list
 /// \return true if and only if the operands lists are identical
 ///
-bool PPCBranchCoalescing::identicalOperands(
+bool BranchCoalescing::identicalOperands(
     ArrayRef<MachineOperand> OpList1, ArrayRef<MachineOperand> OpList2) const {
 
   if (OpList1.size() != OpList2.size()) {
@@ -379,7 +361,7 @@ bool PPCBranchCoalescing::identicalOperands(
 /// \param[in] SourceMBB block to move PHI instructions from
 /// \param[in] TargetMBB block to move PHI instructions to
 ///
-void PPCBranchCoalescing::moveAndUpdatePHIs(MachineBasicBlock *SourceMBB,
+void BranchCoalescing::moveAndUpdatePHIs(MachineBasicBlock *SourceMBB,
                                          MachineBasicBlock *TargetMBB) {
 
   MachineBasicBlock::iterator MI = SourceMBB->begin();
@@ -412,7 +394,7 @@ void PPCBranchCoalescing::moveAndUpdatePHIs(MachineBasicBlock *SourceMBB,
 /// \return true if it is safe to move MI to beginning of TargetMBB,
 ///         false otherwise.
 ///
-bool PPCBranchCoalescing::canMoveToBeginning(const MachineInstr &MI,
+bool BranchCoalescing::canMoveToBeginning(const MachineInstr &MI,
                                           const MachineBasicBlock &TargetMBB
                                           ) const {
 
@@ -443,7 +425,7 @@ bool PPCBranchCoalescing::canMoveToBeginning(const MachineInstr &MI,
 /// \return true if it is safe to move MI to end of TargetMBB,
 ///         false otherwise.
 ///
-bool PPCBranchCoalescing::canMoveToEnd(const MachineInstr &MI,
+bool BranchCoalescing::canMoveToEnd(const MachineInstr &MI,
                                     const MachineBasicBlock &TargetMBB
                                     ) const {
 
@@ -475,7 +457,7 @@ bool PPCBranchCoalescing::canMoveToEnd(const MachineInstr &MI,
 /// \return true if all instructions in SourceRegion.BranchBlock can be merged
 /// into a block in TargetRegion; false otherwise.
 ///
-bool PPCBranchCoalescing::validateCandidates(
+bool BranchCoalescing::validateCandidates(
     CoalescingCandidateInfo &SourceRegion,
     CoalescingCandidateInfo &TargetRegion) const {
 
@@ -518,7 +500,7 @@ bool PPCBranchCoalescing::validateCandidates(
 /// \return true if all instructions in SourceRegion.BranchBlock can be merged
 ///         into a block in TargetRegion, false otherwise.
 ///
-bool PPCBranchCoalescing::canMerge(CoalescingCandidateInfo &SourceRegion,
+bool BranchCoalescing::canMerge(CoalescingCandidateInfo &SourceRegion,
                                 CoalescingCandidateInfo &TargetRegion) const {
   if (!validateCandidates(SourceRegion, TargetRegion))
     return false;
@@ -623,7 +605,7 @@ bool PPCBranchCoalescing::canMerge(CoalescingCandidateInfo &SourceRegion,
 /// \param[in] SourceRegion The candidate to move blocks from
 /// \param[in] TargetRegion The candidate to move blocks to
 ///
-bool PPCBranchCoalescing::mergeCandidates(CoalescingCandidateInfo &SourceRegion,
+bool BranchCoalescing::mergeCandidates(CoalescingCandidateInfo &SourceRegion,
                                        CoalescingCandidateInfo &TargetRegion) {
 
   if (SourceRegion.MustMoveUp && SourceRegion.MustMoveDown) {
@@ -703,9 +685,10 @@ bool PPCBranchCoalescing::mergeCandidates(CoalescingCandidateInfo &SourceRegion,
   return true;
 }
 
-bool PPCBranchCoalescing::runOnMachineFunction(MachineFunction &MF) {
+bool BranchCoalescing::runOnMachineFunction(MachineFunction &MF) {
 
-  if (skipFunction(*MF.getFunction()) || MF.empty())
+  if (skipFunction(*MF.getFunction()) || MF.empty() ||
+      !isBranchCoalescingEnabled())
     return false;
 
   bool didSomething = false;

llvm/lib/CodeGen/CMakeLists.txt

@@ -4,6 +4,7 @@ add_llvm_library(LLVMCodeGen
   Analysis.cpp
   AtomicExpandPass.cpp
   BasicTargetTransformInfo.cpp
+  BranchCoalescing.cpp
   BranchFolding.cpp
   BranchRelaxation.cpp
   BuiltinGCs.cpp

llvm/lib/CodeGen/CodeGen.cpp

@@ -21,6 +21,7 @@ using namespace llvm;
 /// initializeCodeGen - Initialize all passes linked into the CodeGen library.
 void llvm::initializeCodeGen(PassRegistry &Registry) {
   initializeAtomicExpandPass(Registry);
+  initializeBranchCoalescingPass(Registry);
   initializeBranchFolderPassPass(Registry);
   initializeBranchRelaxationPass(Registry);
   initializeCodeGenPreparePass(Registry);

llvm/lib/CodeGen/TargetPassConfig.cpp

@@ -919,6 +919,9 @@ void TargetPassConfig::addMachineSSAOptimization() {
   addPass(&MachineLICMID, false);
   addPass(&MachineCSEID, false);
 
+  // Coalesce basic blocks with the same branch condition
+  addPass(&BranchCoalescingID);
+
   addPass(&MachineSinkingID);
 
   addPass(&PeepholeOptimizerID);

llvm/lib/Target/PowerPC/CMakeLists.txt

@@ -16,7 +16,6 @@ add_llvm_target(PowerPCCodeGen
   PPCBoolRetToInt.cpp
   PPCAsmPrinter.cpp
   PPCBranchSelector.cpp
-  PPCBranchCoalescing.cpp
   PPCCCState.cpp
   PPCCTRLoops.cpp
   PPCHazardRecognizers.cpp

llvm/lib/Target/PowerPC/PPC.h

@@ -41,7 +41,6 @@ namespace llvm {
   FunctionPass *createPPCVSXSwapRemovalPass();
   FunctionPass *createPPCMIPeepholePass();
   FunctionPass *createPPCBranchSelectionPass();
-  FunctionPass *createPPCBranchCoalescingPass();
   FunctionPass *createPPCQPXLoadSplatPass();
   FunctionPass *createPPCISelDag(PPCTargetMachine &TM, CodeGenOpt::Level OL);
   FunctionPass *createPPCTLSDynamicCallPass();

llvm/lib/Target/PowerPC/PPCTargetMachine.cpp

@@ -40,10 +40,6 @@
 
 using namespace llvm;
 
-
-static cl::opt<bool>
-    DisableBranchCoalescing("disable-ppc-branch-coalesce", cl::Hidden,
-                           cl::desc("disable coalescing of duplicate branches for PPC"));
 static cl::
 opt<bool> DisableCTRLoops("disable-ppc-ctrloops", cl::Hidden,
                         cl::desc("Disable CTR loops for PPC"));
@@ -382,10 +378,6 @@ bool PPCPassConfig::addInstSelector() {
 }
 
 void PPCPassConfig::addMachineSSAOptimization() {
-  // PPCBranchCoalescingPass need to be done before machine sinking
-  // since it merges empty blocks.
-  if (!DisableBranchCoalescing && getOptLevel() != CodeGenOpt::None)
-    addPass(createPPCBranchCoalescingPass());
   TargetPassConfig::addMachineSSAOptimization();
   // For little endian, remove where possible the vector swap instructions
   // introduced at code generation to normalize vector element order.

llvm/test/CodeGen/PowerPC/branch_coalesce.ll

@@ -1,10 +1,17 @@
-; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64le-unknown-linux-gnu -verify-machineinstrs -enable-branch-coalesce=true < %s | FileCheck %s 
-; RUN: llc -mcpu=pwr8 -mtriple=powerpc64-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mcpu=pwr8 -mtriple=powerpc64-unknown-linux-gnu -verify-machineinstrs -enable-branch-coalesce=true < %s | FileCheck %s 
-; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -verify-machineinstrs -disable-ppc-branch-coalesce < %s | FileCheck --check-prefix=CHECK-NOCOALESCE %s
-; RUN: llc -mcpu=pwr8 -mtriple=powerpc64-unknown-linux-gnu -verify-machineinstrs -disable-ppc-branch-coalesce < %s | FileCheck --check-prefix=CHECK-NOCOALESCE %s
 
 ; Function Attrs: nounwind
 define double @testBranchCoal(double %a, double %b, double %c, i32 %x) {
+entry:
+  %test = icmp eq i32 %x, 0
+  %tmp1 = select i1 %test, double %a, double 2.000000e-03
+  %tmp2 = select i1 %test, double %b, double 0.000000e+00
+  %tmp3 = select i1 %test, double %c, double 5.000000e-03
+
+  %res1 = fadd double %tmp1, %tmp2
+  %result = fadd double %res1, %tmp3
+  ret double %result
 
 ; CHECK-LABEL: @testBranchCoal 
 ; CHECK: cmplwi [[CMPR:[0-7]+]], 6, 0
@@ -21,40 +28,4 @@ define double @testBranchCoal(double %a, double %b, double %c, i32 %x) {
 ; CHECK: xsadddp 0, 1, 2
 ; CHECK: xsadddp 1, 0, 3
 ; CHECK: blr
-
-; CHECK-NOCOALESCE-LABEL: testBranchCoal:
-; CHECK-NOCOALESCE:       # BB#0: # %entry
-; CHECK-NOCOALESCE-NEXT:    cmplwi 0, 6, 0
-; CHECK-NOCOALESCE-NEXT:    bne 0, .LBB0_5
-; CHECK-NOCOALESCE-NEXT:  # BB#1: # %entry
-; CHECK-NOCOALESCE-NEXT:    bne 0, .LBB0_6
-; CHECK-NOCOALESCE-NEXT:  .LBB0_2: # %entry
-; CHECK-NOCOALESCE-NEXT:    beq 0, .LBB0_4
-; CHECK-NOCOALESCE-NEXT:  .LBB0_3: # %entry
-; CHECK-NOCOALESCE-NEXT:    addis 3, 2, .LCPI0_1@toc@ha
-; CHECK-NOCOALESCE-NEXT:    addi 3, 3, .LCPI0_1@toc@l
-; CHECK-NOCOALESCE-NEXT:    lxsdx 3, 0, 3
-; CHECK-NOCOALESCE-NEXT:  .LBB0_4: # %entry
-; CHECK-NOCOALESCE-NEXT:    xsadddp 0, 1, 2
-; CHECK-NOCOALESCE-NEXT:    xsadddp 1, 0, 3
-; CHECK-NOCOALESCE-NEXT:    blr
-; CHECK-NOCOALESCE-NEXT:  .LBB0_5: # %entry
-; CHECK-NOCOALESCE-NEXT:    addis 3, 2, .LCPI0_0@toc@ha
-; CHECK-NOCOALESCE-NEXT:    addi 3, 3, .LCPI0_0@toc@l
-; CHECK-NOCOALESCE-NEXT:    lxsdx 1, 0, 3
-; CHECK-NOCOALESCE-NEXT:    beq 0, .LBB0_2
-; CHECK-NOCOALESCE-NEXT:  .LBB0_6: # %entry
-; CHECK-NOCOALESCE-NEXT:    xxlxor 2, 2, 2
-; CHECK-NOCOALESCE-NEXT:    bne 0, .LBB0_3
-; CHECK-NOCOALESCE-NEXT:    b .LBB0_4
-  entry:
-
-  %test = icmp eq i32 %x, 0
-  %tmp1 = select i1 %test, double %a, double 2.000000e-03
-  %tmp2 = select i1 %test, double %b, double 0.000000e+00
-  %tmp3 = select i1 %test, double %c, double 5.000000e-03
-
-  %res1 = fadd double %tmp1, %tmp2
-  %result = fadd double %res1, %tmp3
-  ret double %result
 }

llvm/test/CodeGen/PowerPC/select-i1-vs-i1.ll

@@ -1026,6 +1026,10 @@ entry:
   %cond = select i1 %cmp3, ppc_fp128 %a1, ppc_fp128 %a2
   ret ppc_fp128 %cond
 
+; FIXME: Because of the way that the late SELECT_* pseudo-instruction expansion
+; works, we end up with two blocks with the same predicate. These could be
+; combined.
+
 ; CHECK-LABEL: @testppc_fp128eq
 ; CHECK-DAG: fcmpu {{[0-9]+}}, 6, 8
 ; CHECK-DAG: fcmpu {{[0-9]+}}, 5, 7
@@ -1036,8 +1040,10 @@ entry:
 ; CHECK: crxor [[REG3:[0-9]+]], [[REG2]], [[REG1]]
 ; CHECK: bc 12, [[REG3]], .LBB[[BB1:[0-9_]+]]
 ; CHECK: fmr 11, 9
-; CHECK: fmr 12, 10
 ; CHECK: .LBB[[BB1]]:
+; CHECK: bc 12, [[REG3]], .LBB[[BB2:[0-9_]+]]
+; CHECK: fmr 12, 10
+; CHECK: .LBB[[BB2]]:
 ; CHECK-DAG: fmr 1, 11
 ; CHECK-DAG: fmr 2, 12
 ; CHECK: blr