[MBP] Split block placement and block alignment. NFC.

[haicheng]

Summary

Cut and paste.

[deadalnix]

LGTM. Please give it some time for other to comment before landing.

lib/CodeGen/MachineBlockPlacement.cpp

1206

I assume this is the next step.

[haicheng]

lib/CodeGen/MachineBlockPlacement.cpp

1206

Thank you, Amaury. The fix is in D19741.

[haicheng]

Committed in r268067. Thank you.

lib/CodeGen/MachineBlockPlacement.cpp

MachineBasicBlock *findBestLoopExit(MachineFunction &F, MachineLoop &L,
                                       const BlockFilterSet &LoopBlockSet);
   BlockFilterSet collectLoopBlockSet(MachineFunction &F, MachineLoop &L);
   void buildLoopChains(MachineFunction &F, MachineLoop &L);
   void rotateLoop(BlockChain &LoopChain, MachineBasicBlock *ExitingBB,
                   const BlockFilterSet &LoopBlockSet);
   void rotateLoopWithProfile(BlockChain &LoopChain, MachineLoop &L,
                              const BlockFilterSet &LoopBlockSet);
   void buildCFGChains(MachineFunction &F);
+  void alignBlocks(MachineFunction &F);
 
 public:
   static char ID; // Pass identification, replacement for typeid
   MachineBlockPlacement() : MachineFunctionPass(ID) {
     initializeMachineBlockPlacementPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnMachineFunction(MachineFunction &F) override;
if (!LoopBlockSet.empty()) {
                << "  Loop header:  " << getBlockName(*L.block_begin()) << "\n"
                << "  Chain header: " << getBlockName(*LoopChain.begin()) << "\n"
                << "  Bad block:    " << getBlockName(LoopBB) << "\n";
     }
     assert(!BadLoop && "Detected problems with the placement of this loop.");
   });
 }
 
+void MachineBlockPlacement::alignBlocks(MachineFunction &F) {
+  // Walk through the backedges of the function now that we have fully laid out
+  // the basic blocks and align the destination of each backedge. We don't rely
+  // exclusively on the loop info here so that we can align backedges in
+  // unnatural CFGs and backedges that were introduced purely because of the
+  // loop rotations done during this layout pass.
+  // FIXME: Use Function::optForSize().
+  if (F.getFunction()->hasFnAttribute(Attribute::OptimizeForSize))
+    return;
+  BlockChain &FunctionChain = *BlockToChain[&F.front()];
+  if (FunctionChain.begin() == FunctionChain.end())
+    return; // Empty chain.
+
+  const BranchProbability ColdProb(1, 5); // 20%
+  BlockFrequency EntryFreq = MBFI->getBlockFreq(&F.front());
+  BlockFrequency WeightedEntryFreq = EntryFreq * ColdProb;
+  for (MachineBasicBlock *ChainBB : FunctionChain) {
+    if (ChainBB == *FunctionChain.begin())
+      continue;
+
+    // Don't align non-looping basic blocks. These are unlikely to execute
+    // enough times to matter in practice. Note that we'll still handle
+    // unnatural CFGs inside of a natural outer loop (the common case) and
+    // rotated loops.
+    MachineLoop *L = MLI->getLoopFor(ChainBB);
+    if (!L)
+      continue;
+
+    unsigned Align = TLI->getPrefLoopAlignment(L);
+    if (!Align)
+      continue; // Don't care about loop alignment.
+
+    // If the block is cold relative to the function entry don't waste space
+    // aligning it.
+    BlockFrequency Freq = MBFI->getBlockFreq(ChainBB);
+    if (Freq < WeightedEntryFreq)
+      continue;
+
+    // If the block is cold relative to its loop header, don't align it
+    // regardless of what edges into the block exist.
+    MachineBasicBlock *LoopHeader = L->getHeader();
+    BlockFrequency LoopHeaderFreq = MBFI->getBlockFreq(LoopHeader);
+    if (Freq < (LoopHeaderFreq * ColdProb))
+      continue;
+
+    // Check for the existence of a non-layout predecessor which would benefit
+    // from aligning this block.
+    MachineBasicBlock *LayoutPred =
+        &*std::prev(MachineFunction::iterator(ChainBB));
+
+    // Force alignment if all the predecessors are jumps. We already checked
+    // that the block isn't cold above.
+    if (!LayoutPred->isSuccessor(ChainBB)) {
+      ChainBB->setAlignment(Align);
+      continue;
+    }
+
+    // Align this block if the layout predecessor's edge into this block is
+    // cold relative to the block. When this is true, other predecessors make up
+    // all of the hot entries into the block and thus alignment is likely to be
+    // important.
+    BranchProbability LayoutProb =
+        MBPI->getEdgeProbability(LayoutPred, ChainBB);
+    BlockFrequency LayoutEdgeFreq = MBFI->getBlockFreq(LayoutPred) * LayoutProb;
+    if (LayoutEdgeFreq <= (Freq * ColdProb))
+      ChainBB->setAlignment(Align);
+  }
+}
+
 void MachineBlockPlacement::buildCFGChains(MachineFunction &F) {
   // Ensure that every BB in the function has an associated chain to simplify
   // the assumptions of the remaining algorithm.
   SmallVector<MachineOperand, 4> Cond; // For AnalyzeBranch.
   for (MachineFunction::iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) {
     MachineBasicBlock *BB = &*FI;
     BlockChain *Chain =
         new (ChainAllocator.Allocate()) BlockChain(BlockToChain, BB);
for (MachineBasicBlock *ChainBB : FunctionChain) {
     }
   }
 
   // Fixup the last block.
   Cond.clear();
   MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For AnalyzeBranch.
   if (!TII->AnalyzeBranch(F.back(), TBB, FBB, Cond))
     F.back().updateTerminator();
-
-  // Walk through the backedges of the function now that we have fully laid out
-  // the basic blocks and align the destination of each backedge. We don't rely
-  // exclusively on the loop info here so that we can align backedges in
-  // unnatural CFGs and backedges that were introduced purely because of the
-  // loop rotations done during this layout pass.
-  // FIXME: Use Function::optForSize().
-  if (F.getFunction()->hasFnAttribute(Attribute::OptimizeForSize))
-    return;
-  if (FunctionChain.begin() == FunctionChain.end())
-    return; // Empty chain.
-
-  const BranchProbability ColdProb(1, 5); // 20%
-  BlockFrequency EntryFreq = MBFI->getBlockFreq(&F.front());
-  BlockFrequency WeightedEntryFreq = EntryFreq * ColdProb;
-  for (MachineBasicBlock *ChainBB : FunctionChain) {
-    if (ChainBB == *FunctionChain.begin())
-      continue;
-
-    // Don't align non-looping basic blocks. These are unlikely to execute
-    // enough times to matter in practice. Note that we'll still handle
-    // unnatural CFGs inside of a natural outer loop (the common case) and
-    // rotated loops.
-    MachineLoop *L = MLI->getLoopFor(ChainBB);
-    if (!L)
-      continue;
-
-    unsigned Align = TLI->getPrefLoopAlignment(L);
-    if (!Align)
-      continue; // Don't care about loop alignment.
-
-    // If the block is cold relative to the function entry don't waste space
-    // aligning it.
-    BlockFrequency Freq = MBFI->getBlockFreq(ChainBB);
-    if (Freq < WeightedEntryFreq)
-      continue;
-
-    // If the block is cold relative to its loop header, don't align it
-    // regardless of what edges into the block exist.
-    MachineBasicBlock *LoopHeader = L->getHeader();
-    BlockFrequency LoopHeaderFreq = MBFI->getBlockFreq(LoopHeader);
-    if (Freq < (LoopHeaderFreq * ColdProb))
-      continue;
-
-    // Check for the existence of a non-layout predecessor which would benefit
-    // from aligning this block.
-    MachineBasicBlock *LayoutPred =
-        &*std::prev(MachineFunction::iterator(ChainBB));
-
-    // Force alignment if all the predecessors are jumps. We already checked
-    // that the block isn't cold above.
-    if (!LayoutPred->isSuccessor(ChainBB)) {
-      ChainBB->setAlignment(Align);
-      continue;
-    }
-
-    // Align this block if the layout predecessor's edge into this block is
-    // cold relative to the block. When this is true, other predecessors make up
-    // all of the hot entries into the block and thus alignment is likely to be
-    // important.
-    BranchProbability LayoutProb =
-        MBPI->getEdgeProbability(LayoutPred, ChainBB);
-    BlockFrequency LayoutEdgeFreq = MBFI->getBlockFreq(LayoutPred) * LayoutProb;
-    if (LayoutEdgeFreq <= (Freq * ColdProb))
-      ChainBB->setAlignment(Align);
-  }
 }
 
 bool MachineBlockPlacement::runOnMachineFunction(MachineFunction &F) {
   // Check for single-block functions and skip them.
   if (std::next(F.begin()) == F.end())
     return false;
 
   if (skipFunction(*F.getFunction()))
     return false;
 
   MBPI = &getAnalysis<MachineBranchProbabilityInfo>();
   MBFI = &getAnalysis<MachineBlockFrequencyInfo>();
   MLI = &getAnalysis<MachineLoopInfo>();
   TII = F.getSubtarget().getInstrInfo();
   TLI = F.getSubtarget().getTargetLowering();
   MDT = &getAnalysis<MachineDominatorTree>();
   assert(BlockToChain.empty());
 
   buildCFGChains(F);
+  alignBlocks(F);
 
   BlockToChain.clear();
   ChainAllocator.DestroyAll();
 
   if (AlignAllBlock)
     // Align all of the blocks in the function to a specific alignment.
     for (MachineBasicBlock &MBB : F)
       MBB.setAlignment(AlignAllBlock);