Skip to content

Commit

Permalink
[BOLT] Avoid pointless loop rotation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
spupyrev committed Mar 22, 2022
1 parent 0394916 commit 4609f60
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
13 changes: 9 additions & 4 deletions bolt/lib/Passes/LoopInversionPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions bolt/test/X86/loop-inversion-pass.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4609f60

Please sign in to comment.