[MBP] Disable aggressive loop rotate in plain mode

Patch https://reviews.llvm.org/D43256 introduced more aggressive loop layout optimization which depends on profile information. If profile information is not available, the statically estimated profile information(generated by BranchProbabilityInfo.cpp) is used. If user program doesn't behave as BranchProbabilityInfo.cpp expected, the layout may be worse.

To be conservative this patch restores the original layout algorithm in plain mode. But user can still try the aggressive layout optimization with -force-precise-rotation-cost=true.

Differential Revision: https://reviews.llvm.org/D65673

llvm-svn: 369664

Author: weiguozhi

llvm/lib/CodeGen/MachineBlockPlacement.cpp

@@ -462,17 +462,20 @@ class MachineBlockPlacement : public MachineFunctionPass {
                                   const MachineBasicBlock *ExitBB,
                                   const BlockFilterSet &LoopBlockSet);
   MachineBasicBlock *findBestLoopTopHelper(MachineBasicBlock *OldTop,
-      const MachineLoop &L, const BlockFilterSet &LoopBlockSet);
+                                           const MachineLoop &L,
+                                           const BlockFilterSet &LoopBlockSet,
+                                           bool HasStaticProfileOnly = false);
   MachineBasicBlock *findBestLoopTop(
       const MachineLoop &L, const BlockFilterSet &LoopBlockSet);
+  MachineBasicBlock *findBestLoopTopNoProfile(
+      const MachineLoop &L, const BlockFilterSet &LoopBlockSet);
   MachineBasicBlock *findBestLoopExit(
-      const MachineLoop &L, const BlockFilterSet &LoopBlockSet,
-      BlockFrequency &ExitFreq);
+      const MachineLoop &L, const BlockFilterSet &LoopBlockSet);
   BlockFilterSet collectLoopBlockSet(const MachineLoop &L);
   void buildLoopChains(const MachineLoop &L);
   void rotateLoop(
       BlockChain &LoopChain, const MachineBasicBlock *ExitingBB,
-      BlockFrequency ExitFreq, const BlockFilterSet &LoopBlockSet);
+      const BlockFilterSet &LoopBlockSet);
   void rotateLoopWithProfile(
       BlockChain &LoopChain, const MachineLoop &L,
       const BlockFilterSet &LoopBlockSet);
@@ -1947,11 +1950,14 @@ MachineBlockPlacement::FallThroughGains(
 ///        At the same time, move it before old top increases the taken branch
 ///        to loop exit block, so the reduced taken branch will be compared with
 ///        the increased taken branch to the loop exit block.
+///
+///        This pattern is enabled only when HasStaticProfileOnly is false.
 MachineBasicBlock *
 MachineBlockPlacement::findBestLoopTopHelper(
     MachineBasicBlock *OldTop,
     const MachineLoop &L,
-    const BlockFilterSet &LoopBlockSet) {
+    const BlockFilterSet &LoopBlockSet,
+    bool HasStaticProfileOnly) {
   // Check that the header hasn't been fused with a preheader block due to
   // crazy branches. If it has, we need to start with the header at the top to
   // prevent pulling the preheader into the loop body.
@@ -1975,22 +1981,38 @@ MachineBlockPlacement::findBestLoopTopHelper(
     if (Pred->succ_size() > 2)
       continue;
 
-    MachineBasicBlock *OtherBB = nullptr;
-    if (Pred->succ_size() == 2) {
-      OtherBB = *Pred->succ_begin();
-      if (OtherBB == OldTop)
-        OtherBB = *Pred->succ_rbegin();
-    }
-
     if (!canMoveBottomBlockToTop(Pred, OldTop))
       continue;
 
-    BlockFrequency Gains = FallThroughGains(Pred, OldTop, OtherBB,
-                                            LoopBlockSet);
-    if ((Gains > 0) && (Gains > BestGains ||
-        ((Gains == BestGains) && Pred->isLayoutSuccessor(OldTop)))) {
-      BestPred = Pred;
-      BestGains = Gains;
+    if (HasStaticProfileOnly) {
+      // In plain mode we consider pattern 1 only.
+      if (Pred->succ_size() > 1)
+        continue;
+
+      BlockFrequency PredFreq = MBFI->getBlockFreq(Pred);
+      if (!BestPred || PredFreq > BestGains ||
+          (!(PredFreq < BestGains) &&
+           Pred->isLayoutSuccessor(OldTop))) {
+        BestPred = Pred;
+        BestGains = PredFreq;
+      }
+    } else {
+      // With profile information we also consider pattern 2.
+      MachineBasicBlock *OtherBB = nullptr;
+      if (Pred->succ_size() == 2) {
+        OtherBB = *Pred->succ_begin();
+        if (OtherBB == OldTop)
+          OtherBB = *Pred->succ_rbegin();
+      }
+
+      // And more sophisticated cost model.
+      BlockFrequency Gains = FallThroughGains(Pred, OldTop, OtherBB,
+                                              LoopBlockSet);
+      if ((Gains > 0) && (Gains > BestGains ||
+          ((Gains == BestGains) && Pred->isLayoutSuccessor(OldTop)))) {
+        BestPred = Pred;
+        BestGains = Gains;
+      }
     }
   }
 
@@ -2010,7 +2032,7 @@ MachineBlockPlacement::findBestLoopTopHelper(
   return BestPred;
 }
 
-/// Find the best loop top block for layout.
+/// Find the best loop top block for layout in FDO mode.
 ///
 /// This function iteratively calls findBestLoopTopHelper, until no new better
 /// BB can be found.
@@ -2038,15 +2060,42 @@ MachineBlockPlacement::findBestLoopTop(const MachineLoop &L,
   return NewTop;
 }
 
+/// Find the best loop top block for layout in plain mode. It is less agressive
+/// than findBestLoopTop.
+///
+/// Look for a block which is strictly better than the loop header for laying
+/// out at the top of the loop. This looks for one and only one pattern:
+/// a latch block with no conditional exit. This block will cause a conditional
+/// jump around it or will be the bottom of the loop if we lay it out in place,
+/// but if it doesn't end up at the bottom of the loop for any reason,
+/// rotation alone won't fix it. Because such a block will always result in an
+/// unconditional jump (for the backedge) rotating it in front of the loop
+/// header is always profitable.
+MachineBasicBlock *
+MachineBlockPlacement::findBestLoopTopNoProfile(
+    const MachineLoop &L,
+    const BlockFilterSet &LoopBlockSet) {
+  // Placing the latch block before the header may introduce an extra branch
+  // that skips this block the first time the loop is executed, which we want
+  // to avoid when optimising for size.
+  // FIXME: in theory there is a case that does not introduce a new branch,
+  // i.e. when the layout predecessor does not fallthrough to the loop header.
+  // In practice this never happens though: there always seems to be a preheader
+  // that can fallthrough and that is also placed before the header.
+  if (F->getFunction().hasOptSize())
+    return L.getHeader();
+
+  return findBestLoopTopHelper(L.getHeader(), L, LoopBlockSet, true);
+}
+
 /// Find the best loop exiting block for layout.
 ///
 /// This routine implements the logic to analyze the loop looking for the best
 /// block to layout at the top of the loop. Typically this is done to maximize
 /// fallthrough opportunities.
 MachineBasicBlock *
 MachineBlockPlacement::findBestLoopExit(const MachineLoop &L,
-                                        const BlockFilterSet &LoopBlockSet,
-                                        BlockFrequency &ExitFreq) {
+                                        const BlockFilterSet &LoopBlockSet) {
   // We don't want to layout the loop linearly in all cases. If the loop header
   // is just a normal basic block in the loop, we want to look for what block
   // within the loop is the best one to layout at the top. However, if the loop
@@ -2157,7 +2206,6 @@ MachineBlockPlacement::findBestLoopExit(const MachineLoop &L,
 
   LLVM_DEBUG(dbgs() << "  Best exiting block: " << getBlockName(ExitingBB)
                     << "\n");
-  ExitFreq = BestExitEdgeFreq;
   return ExitingBB;
 }
 
@@ -2202,7 +2250,6 @@ MachineBlockPlacement::hasViableTopFallthrough(
 /// of its bottom already, don't rotate it.
 void MachineBlockPlacement::rotateLoop(BlockChain &LoopChain,
                                        const MachineBasicBlock *ExitingBB,
-                                       BlockFrequency ExitFreq,
                                        const BlockFilterSet &LoopBlockSet) {
   if (!ExitingBB)
     return;
@@ -2226,12 +2273,6 @@ void MachineBlockPlacement::rotateLoop(BlockChain &LoopChain,
           (!SuccChain || Succ == *SuccChain->begin()))
         return;
     }
-
-    // Rotate will destroy the top fallthrough, we need to ensure the new exit
-    // frequency is larger than top fallthrough.
-    BlockFrequency FallThrough2Top = TopFallThroughFreq(Top, LoopBlockSet);
-    if (FallThrough2Top >= ExitFreq)
-      return;
   }
 
   BlockChain::iterator ExitIt = llvm::find(LoopChain, ExitingBB);
@@ -2483,7 +2524,10 @@ void MachineBlockPlacement::buildLoopChains(const MachineLoop &L) {
   // loop. This will default to the header, but may end up as one of the
   // predecessors to the header if there is one which will result in strictly
   // fewer branches in the loop body.
-  MachineBasicBlock *LoopTop = findBestLoopTop(L, LoopBlockSet);
+  MachineBasicBlock *LoopTop =
+      (RotateLoopWithProfile || F->getFunction().hasProfileData()) ?
+          findBestLoopTop(L, LoopBlockSet) :
+          findBestLoopTopNoProfile(L, LoopBlockSet);
 
   // If we selected just the header for the loop top, look for a potentially
   // profitable exit block in the event that rotating the loop can eliminate
@@ -2492,9 +2536,8 @@ void MachineBlockPlacement::buildLoopChains(const MachineLoop &L) {
   // Loops are processed innermost to uttermost, make sure we clear
   // PreferredLoopExit before processing a new loop.
   PreferredLoopExit = nullptr;
-  BlockFrequency ExitFreq;
   if (!RotateLoopWithProfile && LoopTop == L.getHeader())
-    PreferredLoopExit = findBestLoopExit(L, LoopBlockSet, ExitFreq);
+    PreferredLoopExit = findBestLoopExit(L, LoopBlockSet);
 
   BlockChain &LoopChain = *BlockToChain[LoopTop];
 
@@ -2511,10 +2554,11 @@ void MachineBlockPlacement::buildLoopChains(const MachineLoop &L) {
 
   buildChain(LoopTop, LoopChain, &LoopBlockSet);
 
-  if (RotateLoopWithProfile)
-    rotateLoopWithProfile(LoopChain, L, LoopBlockSet);
-  else
-    rotateLoop(LoopChain, PreferredLoopExit, ExitFreq, LoopBlockSet);
+  if (RotateLoopWithProfile) {
+    if (LoopTop == L.getHeader())
+      rotateLoopWithProfile(LoopChain, L, LoopBlockSet);
+  } else
+    rotateLoop(LoopChain, PreferredLoopExit, LoopBlockSet);
 
   LLVM_DEBUG({
     // Crash at the end so we get all of the debugging output first.

llvm/test/CodeGen/AArch64/cmpxchg-idioms.ll

@@ -111,7 +111,7 @@ define i1 @test_conditional2(i32 %a, i32 %b, i32* %c) {
 ; CHECK: mov w22, #2
 ; CHECK-NOT: mov w22, #4
 ; CHECK-NOT: cmn w22, #4
-; CHECK: [[LOOP2:LBB[0-9]+_[0-9]+]]: ; %for.cond
+; CHECK: b [[LOOP2:LBB[0-9]+_[0-9]+]]
 ; CHECK-NOT: b.ne [[LOOP2]]
 ; CHECK-NOT: b {{LBB[0-9]+_[0-9]+}}
 ; CHECK: bl _foo

llvm/test/CodeGen/AArch64/tailmerging_in_mbp.ll

@@ -1,8 +1,9 @@
 ; RUN: llc <%s -mtriple=aarch64-eabi -verify-machine-dom-info | FileCheck %s
 
 ; CHECK-LABEL: test:
-; CHECK-LABEL: %cond.false12.i
-; CHECK:         b.gt	
+; CHECK:       LBB0_7:
+; CHECK:         b.hi	
+; CHECK-NEXT:    b	
 ; CHECK-NEXT:  LBB0_8:
 ; CHECK-NEXT:    mov	 x8, x9
 ; CHECK-NEXT:  LBB0_9:

llvm/test/CodeGen/AMDGPU/collapse-endcf.ll

@@ -230,11 +230,6 @@ bb.end:                                           ; preds = %bb.then, %bb
 ; Make sure scc liveness is updated if sor_b64 is removed
 ; ALL-LABEL: {{^}}scc_liveness:
 
-; GCN: %bb10
-; GCN: s_or_b64 exec, exec, s{{\[[0-9]+:[0-9]+\]}}
-; GCN: s_andn2_b64
-; GCN-NEXT: s_cbranch_execz
-
 ; GCN: [[BB1_LOOP:BB[0-9]+_[0-9]+]]:
 ; GCN: s_andn2_b64 exec, exec,
 ; GCN-NEXT: s_cbranch_execnz [[BB1_LOOP]]
@@ -244,6 +239,10 @@ bb.end:                                           ; preds = %bb.then, %bb
 
 ; GCN-NOT: s_or_b64 exec, exec
 
+; GCN: s_or_b64 exec, exec, s{{\[[0-9]+:[0-9]+\]}}
+; GCN: s_andn2_b64
+; GCN-NEXT: s_cbranch_execnz
+
 ; GCN: s_or_b64 exec, exec, s{{\[[0-9]+:[0-9]+\]}}
 ; GCN: buffer_store_dword
 ; GCN: buffer_store_dword

llvm/test/CodeGen/AMDGPU/divergent-branch-uniform-condition.ll

@@ -20,41 +20,38 @@ define amdgpu_ps void @main(i32, float) {
 ; CHECK-NEXT:    ; implicit-def: $sgpr8_sgpr9
 ; CHECK-NEXT:    ; implicit-def: $sgpr6_sgpr7
 ; CHECK-NEXT:    ; implicit-def: $sgpr2_sgpr3
-; CHECK-NEXT:    s_branch BB0_3
-; CHECK-NEXT:  BB0_1: ; %Flow1
-; CHECK-NEXT:    ; in Loop: Header=BB0_3 Depth=1
-; CHECK-NEXT:    s_or_b64 exec, exec, s[8:9]
-; CHECK-NEXT:    s_mov_b64 s[8:9], 0
-; CHECK-NEXT:  BB0_2: ; %Flow
-; CHECK-NEXT:    ; in Loop: Header=BB0_3 Depth=1
-; CHECK-NEXT:    s_and_b64 s[10:11], exec, s[6:7]
-; CHECK-NEXT:    s_or_b64 s[10:11], s[10:11], s[4:5]
-; CHECK-NEXT:    s_andn2_b64 s[2:3], s[2:3], exec
-; CHECK-NEXT:    s_and_b64 s[4:5], s[8:9], exec
-; CHECK-NEXT:    s_or_b64 s[2:3], s[2:3], s[4:5]
-; CHECK-NEXT:    s_mov_b64 s[4:5], s[10:11]
-; CHECK-NEXT:    s_andn2_b64 exec, exec, s[10:11]
-; CHECK-NEXT:    s_cbranch_execz BB0_6
-; CHECK-NEXT:  BB0_3: ; %loop
+; CHECK-NEXT:  BB0_1: ; %loop
 ; CHECK-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; CHECK-NEXT:    v_cmp_gt_u32_e32 vcc, 32, v1
 ; CHECK-NEXT:    s_and_b64 vcc, exec, vcc
 ; CHECK-NEXT:    s_or_b64 s[6:7], s[6:7], exec
 ; CHECK-NEXT:    s_or_b64 s[8:9], s[8:9], exec
-; CHECK-NEXT:    s_cbranch_vccz BB0_2
-; CHECK-NEXT:  ; %bb.4: ; %endif1
-; CHECK-NEXT:    ; in Loop: Header=BB0_3 Depth=1
+; CHECK-NEXT:    s_cbranch_vccz BB0_5
+; CHECK-NEXT:  ; %bb.2: ; %endif1
+; CHECK-NEXT:    ; in Loop: Header=BB0_1 Depth=1
 ; CHECK-NEXT:    s_mov_b64 s[6:7], -1
 ; CHECK-NEXT:    s_and_saveexec_b64 s[8:9], s[0:1]
 ; CHECK-NEXT:    s_xor_b64 s[8:9], exec, s[8:9]
-; CHECK-NEXT:    ; mask branch BB0_1
-; CHECK-NEXT:    s_cbranch_execz BB0_1
-; CHECK-NEXT:  BB0_5: ; %endif2
-; CHECK-NEXT:    ; in Loop: Header=BB0_3 Depth=1
+; CHECK-NEXT:    ; mask branch BB0_4
+; CHECK-NEXT:  BB0_3: ; %endif2
+; CHECK-NEXT:    ; in Loop: Header=BB0_1 Depth=1
 ; CHECK-NEXT:    v_add_u32_e32 v1, 1, v1
 ; CHECK-NEXT:    s_xor_b64 s[6:7], exec, -1
-; CHECK-NEXT:    s_branch BB0_1
-; CHECK-NEXT:  BB0_6: ; %Flow2
+; CHECK-NEXT:  BB0_4: ; %Flow1
+; CHECK-NEXT:    ; in Loop: Header=BB0_1 Depth=1
+; CHECK-NEXT:    s_or_b64 exec, exec, s[8:9]
+; CHECK-NEXT:    s_mov_b64 s[8:9], 0
+; CHECK-NEXT:  BB0_5: ; %Flow
+; CHECK-NEXT:    ; in Loop: Header=BB0_1 Depth=1
+; CHECK-NEXT:    s_and_b64 s[10:11], exec, s[6:7]
+; CHECK-NEXT:    s_or_b64 s[10:11], s[10:11], s[4:5]
+; CHECK-NEXT:    s_andn2_b64 s[2:3], s[2:3], exec
+; CHECK-NEXT:    s_and_b64 s[4:5], s[8:9], exec
+; CHECK-NEXT:    s_or_b64 s[2:3], s[2:3], s[4:5]
+; CHECK-NEXT:    s_mov_b64 s[4:5], s[10:11]
+; CHECK-NEXT:    s_andn2_b64 exec, exec, s[10:11]
+; CHECK-NEXT:    s_cbranch_execnz BB0_1
+; CHECK-NEXT:  ; %bb.6: ; %Flow2
 ; CHECK-NEXT:    s_or_b64 exec, exec, s[10:11]
 ; CHECK-NEXT:    v_mov_b32_e32 v1, 0
 ; CHECK-NEXT:    s_and_saveexec_b64 s[0:1], s[2:3]
@@ -65,7 +62,6 @@ define amdgpu_ps void @main(i32, float) {
 ; CHECK-NEXT:    s_or_b64 exec, exec, s[0:1]
 ; CHECK-NEXT:    exp mrt0 v1, v1, v1, v1 done vm
 ; CHECK-NEXT:    s_endpgm
-; this is the divergent branch with the condition not marked as divergent
 start:
   %v0 = call float @llvm.amdgcn.interp.p1(float %1, i32 0, i32 0, i32 %0)
   br label %loop

llvm/test/CodeGen/AMDGPU/global_smrd_cfg.ll

@@ -1,28 +1,27 @@
 ; RUN: llc -mtriple amdgcn--amdhsa -mcpu=fiji -amdgpu-scalarize-global-loads=true -verify-machineinstrs  < %s | FileCheck %s
 
-; CHECK-LABEL: %bb22
+; CHECK-LABEL: %bb11
 
-; Load from %arg has alias store in Loop
+; Load from %arg in a Loop body has alias store
 
 ; CHECK: flat_load_dword
 
-; #####################################################################
-
-; Load from %arg1 has no-alias store in Loop - arg1[i+1] never alias arg1[i]
-
-; CHECK: s_load_dword
+; CHECK-LABEL: %bb20
+; CHECK: flat_store_dword
 
 ; #####################################################################
 
-; CHECK-LABEL: %bb11
+; CHECK-LABEL: %bb22
 
-; Load from %arg in a Loop body has alias store
+; Load from %arg has alias store in Loop
 
 ; CHECK: flat_load_dword
 
-; CHECK-LABEL: %bb20
+; #####################################################################
 
-; CHECK: flat_store_dword
+; Load from %arg1 has no-alias store in Loop - arg1[i+1] never alias arg1[i]
+
+; CHECK: s_load_dword
 
 define amdgpu_kernel void @cfg(i32 addrspace(1)* nocapture readonly %arg, i32 addrspace(1)* nocapture %arg1, i32 %arg2) #0 {
 bb:

llvm/test/CodeGen/AMDGPU/i1-copy-from-loop.ll

@@ -3,20 +3,20 @@
 
 ; SI-LABEL: {{^}}i1_copy_from_loop:
 ;
-; SI: ; %Flow
-; SI-DAG:  s_andn2_b64       [[LCSSA_ACCUM:s\[[0-9]+:[0-9]+\]]], [[LCSSA_ACCUM]], exec
-; SI-DAG:  s_and_b64         [[CC_MASK2:s\[[0-9]+:[0-9]+\]]], [[CC_ACCUM:s\[[0-9]+:[0-9]+\]]], exec
-; SI:      s_or_b64          [[LCSSA_ACCUM]], [[LCSSA_ACCUM]], [[CC_MASK2]]
-
 ; SI: ; %for.body
 ; SI:      v_cmp_gt_u32_e64  [[CC_SREG:s\[[0-9]+:[0-9]+\]]], 4,
-; SI-DAG:  s_andn2_b64       [[CC_ACCUM]], [[CC_ACCUM]], exec
+; SI-DAG:  s_andn2_b64       [[CC_ACCUM:s\[[0-9]+:[0-9]+\]]], [[CC_ACCUM]], exec
 ; SI-DAG:  s_and_b64         [[CC_MASK:s\[[0-9]+:[0-9]+\]]], [[CC_SREG]], exec
 ; SI:      s_or_b64          [[CC_ACCUM]], [[CC_ACCUM]], [[CC_MASK]]
 
 ; SI: ; %Flow1
 ; SI:      s_or_b64          [[CC_ACCUM]], [[CC_ACCUM]], exec
 
+; SI: ; %Flow
+; SI-DAG:  s_andn2_b64       [[LCSSA_ACCUM:s\[[0-9]+:[0-9]+\]]], [[LCSSA_ACCUM]], exec
+; SI-DAG:  s_and_b64         [[CC_MASK2:s\[[0-9]+:[0-9]+\]]], [[CC_ACCUM]], exec
+; SI:      s_or_b64          [[LCSSA_ACCUM]], [[LCSSA_ACCUM]], [[CC_MASK2]]
+
 ; SI: ; %for.end
 ; SI:      s_and_saveexec_b64 {{s\[[0-9]+:[0-9]+\]}}, [[LCSSA_ACCUM]]