[BOLT] Fix fix-branches in presence of JRCXZ and friends

Summary:
Do not fail/assert when trying to reorder blocks that terminate
with JRCXZ/JECXZ/LOOP instructions. We cannot invert the condition of
these instructions, so just treat them accordingly in fixBranches().

(cherry picked from FBD22487107)
This commit is contained in:
Rafael Auler 2020-07-15 23:02:58 -07:00 committed by Maksim Panchenko
parent 181327d763
commit 170f73ac9e
3 changed files with 65 additions and 8 deletions

View File

@ -3172,7 +3172,10 @@ void BinaryFunction::fixBranches() {
assert(CondBranch && "conditional branch expected");
const auto *TSuccessor = BB->getConditionalSuccessor(true);
const auto *FSuccessor = BB->getConditionalSuccessor(false);
if (NextBB && NextBB == TSuccessor) {
// Check whether we support reversing this branch direction
const auto IsSupported =
!MIB->isUnsupportedBranch(CondBranch->getOpcode());
if (NextBB && NextBB == TSuccessor && IsSupported) {
std::swap(TSuccessor, FSuccessor);
{
auto L = BC.scopeLock();
@ -3186,11 +3189,13 @@ void BinaryFunction::fixBranches() {
if (TSuccessor == FSuccessor) {
BB->removeDuplicateConditionalSuccessor(CondBranch);
}
if (!NextBB || (NextBB != TSuccessor && NextBB != FSuccessor)) {
if (!NextBB ||
((NextBB != TSuccessor || !IsSupported) && NextBB != FSuccessor)) {
// If one of the branches is guaranteed to be "long" while the other
// could be "short", then prioritize short for "taken". This will
// generate a sequence 1 byte shorter on x86.
if (BC.isX86() && TSuccessor->isCold() != FSuccessor->isCold() &&
if (IsSupported && BC.isX86() &&
TSuccessor->isCold() != FSuccessor->isCold() &&
BB->isCold() != TSuccessor->isCold()) {
std::swap(TSuccessor, FSuccessor);
{

View File

@ -2253,11 +2253,7 @@ public:
}
// Handle conditional branches and ignore indirect branches
if (I->getOpcode() != X86::LOOP &&
I->getOpcode() != X86::LOOPE &&
I->getOpcode() != X86::LOOPNE &&
I->getOpcode() != X86::JECXZ &&
I->getOpcode() != X86::JRCXZ &&
if (!isUnsupportedBranch(I->getOpcode()) &&
getInvertedBranchOpcode(I->getOpcode()) == I->getOpcode()) {
// Indirect branch
return false;

View File

@ -0,0 +1,56 @@
# Test BOLT does not crash by trying to change the direction of a JRCXZ
# REQUIRES: system-linux
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
# RUN: %s -o %t.o
# RUN: link_fdata %s %t.o %t.fdata
# RUN: strip --strip-unneeded %t.o
# RUN: %host_cc %t.o -o %t.exe -Wl,-q
# RUN: llvm-bolt %t.exe -relocs=1 -reorder-blocks=cache+ -print-finalized \
# RUN: -o %t.out -data %t.fdata | FileCheck %s
# RUN: %t.out 1 2 3
# CHECK: BOLT-INFO
.text
.globl main
.type main, %function
.size main, .Lend1-main
main:
# FDATA: 0 [unknown] 0 1 main 0 0 510
xorq %rcx, %rcx
andq $3, %rdi
jmpq *jumptbl(,%rdi,8)
# Here are the 4 possible targets of the indirect branch to simulate a simple
# CFG. What is important here is that BB1, the first block, conditionally
# transfers control to the exit block with JRCXZ (.J1). We create a mock profile
# saying that this branch is taken more often than not, causing BOLT to try to
# put the exit block after it, which would require us to change the direction
# of JRCXZ.
.BB1:
movl $0x0, %eax
.J1:
jrcxz .BBend
# FDATA: 1 main #.J1# 1 main #.BB2# 0 10
# FDATA: 1 main #.J1# 1 main #.BBend# 0 500
.BB2:
movl $0x2, %eax
jmp .BBend
.Lbb3:
movl $0x3, %eax
jmp .BBend
.Lbb4:
movl $0x4, %eax
.BBend:
retq
.Lend1:
.section .rodata
.globl jumptbl
jumptbl:
.quad .BB1
.quad .BB2
.quad .Lbb3
.quad .Lbb4