From 4609f60ebc57d0bd63f97cae11978b96c53204d6 Mon Sep 17 00:00:00 2001
From: spupyrev <spupyrev@fb.com>
Date: Thu, 17 Mar 2022 09:51:07 -0700
Subject: [PATCH] [BOLT] Avoid pointless loop rotation

It seems the earlier implementation does not follow the description
in LoopRotationPass.h: It rotates loops even if they are already laid out
correctly. The diff adjusts the behaviour.

Given that the impact of LoopInversionPass is minor, this change won't
yield significant perf differences. Tested on clang-10: there seems to be a
0.1%-0.3% cpu win and a small reduction of branch misses.

**Before:**
BOLT-INFO: 120 Functions were reordered by LoopInversionPass

**After:**
BOLT-INFO: 79 Functions were reordered by LoopInversionPass

Reviewed By: yota9

Differential Revision: https://reviews.llvm.org/D121921
---
 bolt/lib/Passes/LoopInversionPass.cpp | 13 +++++++++----
 bolt/test/X86/loop-inversion-pass.s   | 13 +++++++++++--
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/bolt/lib/Passes/LoopInversionPass.cpp b/bolt/lib/Passes/LoopInversionPass.cpp
index 2f32703eb367..30eccb83c7e4 100644
--- a/bolt/lib/Passes/LoopInversionPass.cpp
+++ b/bolt/lib/Passes/LoopInversionPass.cpp
@@ -54,11 +54,16 @@ bool LoopInversionPass::runOnFunction(BinaryFunction &BF) {
         }
       }
 
-      assert(SecondSucc != nullptr && "Unable to find second BB successor");
-      const uint64_t BBCount = SuccBB->getBranchInfo(*BB).Count;
-      const uint64_t OtherCount = SuccBB->getBranchInfo(*SecondSucc).Count;
-      if ((BBCount < OtherCount) && (BBIndex > SuccBBIndex))
+      assert(SecondSucc != nullptr && "Unable to find a second BB successor");
+      const uint64_t LoopCount = SuccBB->getBranchInfo(*BB).Count;
+      const uint64_t ExitCount = SuccBB->getBranchInfo(*SecondSucc).Count;
+
+      if (LoopCount < ExitCount) {
+        if (BBIndex > SuccBBIndex)
+          continue;
+      } else if (BBIndex < SuccBBIndex) {
         continue;
+      }
 
       IsChanged = true;
       BB->setLayoutIndex(SuccBBIndex);
diff --git a/bolt/test/X86/loop-inversion-pass.s b/bolt/test/X86/loop-inversion-pass.s
index bbc7c45c433b..38d3c2af23fe 100644
--- a/bolt/test/X86/loop-inversion-pass.s
+++ b/bolt/test/X86/loop-inversion-pass.s
@@ -4,24 +4,33 @@
 # RUN:   %s -o %t.o
 # RUN: link_fdata %s %t.o %t.fdata
 # RUN: link_fdata %s %t.o %t.fdata2 "FDATA2"
+# RUN: link_fdata %s %t.o %t.fdata3 "FDATA3"
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt %t.exe -data %t.fdata -reorder-blocks=cache+ -print-finalized \
 # RUN:    -loop-inversion-opt -o %t.out | FileCheck %s
 # RUN: llvm-bolt %t.exe -data %t.fdata2 -reorder-blocks=cache+ -print-finalized \
 # RUN:    -loop-inversion-opt -o %t.out2 | FileCheck --check-prefix="CHECK2" %s
+# RUN: llvm-bolt %t.exe -data %t.fdata3 -reorder-blocks=none -print-finalized \
+# RUN:    -loop-inversion-opt -o %t.out3 | FileCheck --check-prefix="CHECK3" %s
 
-# The case where loop is used:
+# The case where the loop is used:
 # FDATA: 1 main 2 1 main #.J1# 0 420
 # FDATA: 1 main b 1 main #.Jloop# 0 420
 # FDATA: 1 main b 1 main d 0 1
 # CHECK: BB Layout   : .LBB00, .Ltmp0, .Ltmp1, .LFT0
 
-# The case where loop is unused:
+# The case where the loop is unused:
 # FDATA2: 1 main 2 1 main #.J1# 0 420
 # FDATA2: 1 main b 1 main #.Jloop# 0 1
 # FDATA2: 1 main b 1 main d 0 420
 # CHECK2: BB Layout   : .LBB00, .Ltmp1, .LFT0, .Ltmp0
 
+# The case where the loop does not require rotation:
+# FDATA3: 1 main 2 1 main #.J1# 0 420
+# FDATA3: 1 main b 1 main #.Jloop# 0 420
+# FDATA3: 1 main b 1 main d 0 1
+# CHECK3: BB Layout   : .LBB00, .Ltmp0, .Ltmp1, .LFT0
+
     .text
     .globl main
     .type main, %function