Skip to content

Commit 00c641f

Browse files
paschalis-mpeispaschalis-mpeis
committed
Refactored convertCallToIndirectCall to createIndirectPltCall
createIndirectPltCall now returns a list of instructions. All BinaryBasicBlock-related logic is now on PLTCall pass. Added the same test for x86, to ensure nothing breaks on that backend.
1 parent 32a1ae3 commit 00c641f

File tree

6 files changed

+67
-67
lines changed

6 files changed

+67
-67
lines changed

‎bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#ifndef BOLT_CORE_MCPLUSBUILDER_H
1515
#define BOLT_CORE_MCPLUSBUILDER_H
1616

17-
#include "bolt/Core/BinaryBasicBlock.h"
1817
#include "bolt/Core/MCPlus.h"
1918
#include "bolt/Core/Relocation.h"
2019
#include "llvm/ADT/ArrayRef.h"
@@ -1413,19 +1412,14 @@ class MCPlusBuilder {
14131412
return false;
14141413
}
14151414

1416-
/// Modify a direct call instruction pointed by the iterator \p It, with an
1417-
/// indirect call taking a destination from a memory location pointed by \p
1418-
/// TargetLocation symbol. If additional instructions need to be prepended
1419-
/// before \p It, then the iterator must be updated to point to the indirect
1420-
/// call instruction.
1421-
///
1422-
/// \return true on success
1423-
virtual bool convertCallToIndirectCall(BinaryBasicBlock &BB,
1424-
BinaryBasicBlock::iterator &It,
1425-
const MCSymbol *TargetLocation,
1426-
MCContext *Ctx) {
1415+
/// Creates an indirect call to the function within the \p DirectCall PLT
1416+
/// stub. The function's memory location is pointed by the \p TargetLocation
1417+
/// symbol.
1418+
virtual InstructionListType
1419+
createIndirectPltCall(const MCInst &DirectCall,
1420+
const MCSymbol *TargetLocation, MCContext *Ctx) {
14271421
llvm_unreachable("not implemented");
1428-
return false;
1422+
return {};
14291423
}
14301424

14311425
/// Morph an indirect call into a load where \p Reg holds the call target.

‎bolt/lib/Passes/PLTCall.cpp

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ Error PLTCall::runOnFunctions(BinaryContext &BC) {
4848
return Error::success();
4949

5050
uint64_t NumCallsOptimized = 0;
51-
for (auto &It : BC.getBinaryFunctions()) {
52-
BinaryFunction &Function = It.second;
51+
for (auto &BFI : BC.getBinaryFunctions()) {
52+
BinaryFunction &Function = BFI.second;
5353
if (!shouldOptimize(Function))
5454
continue;
5555

@@ -61,23 +61,21 @@ Error PLTCall::runOnFunctions(BinaryContext &BC) {
6161
if (opts::PLT == OT_HOT && !BB.getKnownExecutionCount())
6262
continue;
6363

64-
for (auto It = BB.begin(); It != BB.end(); It++) {
65-
if (!BC.MIB->isCall(*It))
64+
for (auto II = BB.begin(); II != BB.end(); II++) {
65+
if (!BC.MIB->isCall(*II))
6666
continue;
67-
const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(*It);
67+
const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(*II);
6868
if (!CallSymbol)
6969
continue;
7070
const BinaryFunction *CalleeBF = BC.getFunctionForSymbol(CallSymbol);
7171
if (!CalleeBF || !CalleeBF->isPLTFunction())
7272
continue;
73-
if (BC.MIB->convertCallToIndirectCall(BB, It, CalleeBF->getPLTSymbol(),
74-
BC.Ctx.get())) {
75-
assert(BC.MIB->isCall(*It) &&
76-
"Iterator must point to the optimized call");
77-
78-
BC.MIB->addAnnotation(*It, "PLTCall", true);
79-
++NumCallsOptimized;
80-
}
73+
const InstructionListType NewCode = BC.MIB->createIndirectPltCall(
74+
*II, CalleeBF->getPLTSymbol(), BC.Ctx.get());
75+
II = BB.replaceInstruction(II, NewCode);
76+
std::advance(II, NewCode.size() - 1);
77+
BC.MIB->addAnnotation(*II, "PLTCall", true);
78+
++NumCallsOptimized;
8179
}
8280
}
8381
}

‎bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,29 +1055,27 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
10551055
return true;
10561056
}
10571057

1058-
bool convertCallToIndirectCall(BinaryBasicBlock &BB,
1059-
BinaryBasicBlock::iterator &It,
1060-
const MCSymbol *TargetLocation,
1061-
MCContext *Ctx) override {
1062-
// Generated code:
1063-
// adrp x16 <symbol>
1064-
// ldr x17, [x16, #<offset>]
1065-
// bl <label> -> blr x17 (or covert 'b -> br' for tail calls)
1066-
1067-
MCInst &InstCall = *It;
1068-
bool IsTailCall = isTailCall(InstCall);
1069-
assert((InstCall.getOpcode() == AArch64::BL ||
1070-
(InstCall.getOpcode() == AArch64::B && IsTailCall)) &&
1058+
InstructionListType createIndirectPltCall(const MCInst &DirectCall,
1059+
const MCSymbol *TargetLocation,
1060+
MCContext *Ctx) override {
1061+
const bool IsTailCall = isTailCall(DirectCall);
1062+
assert((DirectCall.getOpcode() == AArch64::BL ||
1063+
(DirectCall.getOpcode() == AArch64::B && IsTailCall)) &&
10711064
"64-bit direct (tail) call instruction expected");
10721065

1073-
// Convert the call to an indicrect one by modifying the instruction.
1074-
InstCall.clear();
1075-
InstCall.setOpcode(IsTailCall ? AArch64::BR : AArch64::BLR);
1076-
InstCall.addOperand(MCOperand::createReg(AArch64::X17));
1077-
if (IsTailCall)
1078-
setTailCall(*It);
1066+
InstructionListType Code;
1067+
// Code sequence for indirect plt call:
1068+
// adrp x16 <symbol>
1069+
// ldr x17, [x16, #<offset>]
1070+
// blr x17 ; or 'br' for tail calls
10791071

1080-
// Prepend instructions to load PLT call address from the input symbol.
1072+
MCInst InstAdrp;
1073+
InstAdrp.setOpcode(AArch64::ADRP);
1074+
InstAdrp.addOperand(MCOperand::createReg(AArch64::X16));
1075+
InstAdrp.addOperand(MCOperand::createImm(0));
1076+
setOperandToSymbolRef(InstAdrp, /* OpNum */ 1, TargetLocation,
1077+
/* Addend */ 0, Ctx, ELF::R_AARCH64_ADR_GOT_PAGE);
1078+
Code.emplace_back(InstAdrp);
10811079

10821080
MCInst InstLoad;
10831081
InstLoad.setOpcode(AArch64::LDRXui);
@@ -1086,19 +1084,16 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
10861084
InstLoad.addOperand(MCOperand::createImm(0));
10871085
setOperandToSymbolRef(InstLoad, /* OpNum */ 2, TargetLocation,
10881086
/* Addend */ 0, Ctx, ELF::R_AARCH64_LD64_GOT_LO12_NC);
1089-
It = BB.insertInstruction(It, InstLoad);
1087+
Code.emplace_back(InstLoad);
10901088

1091-
MCInst InstAdrp;
1092-
InstAdrp.setOpcode(AArch64::ADRP);
1093-
InstAdrp.clear();
1094-
InstAdrp.addOperand(MCOperand::createReg(AArch64::X16));
1095-
InstAdrp.addOperand(MCOperand::createImm(0));
1096-
setOperandToSymbolRef(InstAdrp, /* OpNum */ 1, TargetLocation,
1097-
/* Addend */ 0, Ctx, ELF::R_AARCH64_ADR_GOT_PAGE);
1098-
It = BB.insertInstruction(It, InstAdrp);
1089+
MCInst InstCall;
1090+
InstCall.setOpcode(IsTailCall ? AArch64::BR : AArch64::BLR);
1091+
InstCall.addOperand(MCOperand::createReg(AArch64::X17));
1092+
if (IsTailCall)
1093+
setTailCall(InstCall);
1094+
Code.emplace_back(InstCall);
10991095

1100-
It = It + 2;
1101-
return true;
1096+
return Code;
11021097
}
11031098

11041099
bool lowerTailCall(MCInst &Inst) override {

‎bolt/lib/Target/X86/X86MCPlusBuilder.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,14 +1639,16 @@ class X86MCPlusBuilder : public MCPlusBuilder {
16391639
return true;
16401640
}
16411641

1642-
bool convertCallToIndirectCall(BinaryBasicBlock &BB,
1643-
BinaryBasicBlock::iterator &It,
1644-
const MCSymbol *TargetLocation,
1645-
MCContext *Ctx) override {
1646-
MCInst &Inst = (*It);
1647-
assert((Inst.getOpcode() == X86::CALL64pcrel32 ||
1648-
(Inst.getOpcode() == X86::JMP_4 && isTailCall(Inst))) &&
1642+
InstructionListType createIndirectPltCall(const MCInst &DirectCall,
1643+
const MCSymbol *TargetLocation,
1644+
MCContext *Ctx) override {
1645+
assert((DirectCall.getOpcode() == X86::CALL64pcrel32 ||
1646+
(DirectCall.getOpcode() == X86::JMP_4 && isTailCall(DirectCall))) &&
16491647
"64-bit direct (tail) call instruction expected");
1648+
1649+
InstructionListType Code;
1650+
// Create a new indirect call by converting the previous direct call.
1651+
MCInst Inst = DirectCall;
16501652
const auto NewOpcode =
16511653
(Inst.getOpcode() == X86::CALL64pcrel32) ? X86::CALL64m : X86::JMP32m;
16521654
Inst.setOpcode(NewOpcode);
@@ -1667,7 +1669,8 @@ class X86MCPlusBuilder : public MCPlusBuilder {
16671669
Inst.insert(Inst.begin(),
16681670
MCOperand::createReg(X86::RIP)); // BaseReg
16691671

1670-
return true;
1672+
Code.emplace_back(Inst);
1673+
return Code;
16711674
}
16721675

16731676
void convertIndirectCallToLoad(MCInst &Inst, MCPhysReg Reg) override {

‎bolt/test/AArch64/plt-call.test

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
// Verify that PLTCall optimization works, including when PLT calls were
2-
// tail-call optimized.
1+
// Verify that PLTCall optimization works.
32

43
RUN: %clang %cflags %p/../Inputs/plt-tailcall.c \
54
RUN: -o %t -Wl,-q

‎bolt/test/X86/plt-call.test

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Verify that PLTCall optimization works.
2+
3+
RUN: %clang %cflags %p/../Inputs/plt-tailcall.c \
4+
RUN: -o %t -Wl,-q
5+
RUN: llvm-bolt %t -o %t.bolt --plt=all --print-plt --print-only=foo | FileCheck %s
6+
7+
// Call to printf
8+
CHECK: callq *printf@GOT(%rip) # PLTCall: 1
9+
10+
// Call to puts, that was tail-call optimized
11+
CHECK: jmpl *puts@GOT(%rip) # TAILCALL # PLTCall: 1

0 commit comments

Comments
 (0)