Skip to content

Commit c257b80

Browse files
ElvinaYakubovaElvinaYakubova
committed
[BOLT][AArch64] Skip only BBs during instrumentation
1 parent b3ae6c2 commit c257b80

File tree

4 files changed

+194
-39
lines changed

4 files changed

+194
-39
lines changed

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,17 @@ class MCPlusBuilder {
620620
return Info->get(Inst.getOpcode()).mayStore();
621621
}
622622

623-
virtual bool isAArch64Exclusive(const MCInst &Inst) const {
623+
virtual bool isAArch64ExclusiveLoad(const MCInst &Inst) const {
624+
llvm_unreachable("not implemented");
625+
return false;
626+
}
627+
628+
virtual bool isAArch64ExclusiveStore(const MCInst &Inst) const {
629+
llvm_unreachable("not implemented");
630+
return false;
631+
}
632+
633+
virtual bool isAArch64ExclusiveClear(const MCInst &Inst) const {
624634
llvm_unreachable("not implemented");
625635
return false;
626636
}

‎bolt/lib/Passes/Instrumentation.cpp

Lines changed: 84 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,33 +86,90 @@ cl::opt<bool> InstrumentCalls("instrument-calls",
8686
namespace llvm {
8787
namespace bolt {
8888

89-
static bool hasAArch64ExclusiveMemop(BinaryFunction &Function) {
89+
static bool hasAArch64ExclusiveMemop(
90+
BinaryFunction &Function,
91+
std::unordered_set<const BinaryBasicBlock *> &BBToSkip) {
9092
// FIXME ARMv8-a architecture reference manual says that software must avoid
9193
// having any explicit memory accesses between exclusive load and associated
92-
// store instruction. So for now skip instrumentation for functions that have
93-
// these instructions, since it might lead to runtime deadlock.
94+
// store instruction. So for now skip instrumentation for basic blocks that
95+
// have these instructions, since it might lead to runtime deadlock.
9496
BinaryContext &BC = Function.getBinaryContext();
95-
for (const BinaryBasicBlock &BB : Function)
96-
for (const MCInst &Inst : BB)
97-
if (BC.MIB->isAArch64Exclusive(Inst)) {
98-
if (opts::Verbosity >= 1)
99-
BC.outs() << "BOLT-INSTRUMENTER: Function " << Function
100-
<< " has exclusive instructions, skip instrumentation\n";
97+
std::queue<std::pair<BinaryBasicBlock *, bool>> BBQueue; // {BB, isLoad}
98+
std::unordered_set<BinaryBasicBlock *> Visited;
99+
100+
if (Function.getLayout().block_begin() == Function.getLayout().block_end())
101+
return 0;
102+
103+
BinaryBasicBlock *BBfirst = *Function.getLayout().block_begin();
104+
BBQueue.push({BBfirst, false});
105+
106+
while (!BBQueue.empty()) {
107+
BinaryBasicBlock *BB = BBQueue.front().first;
108+
bool IsLoad = BBQueue.front().second;
109+
BBQueue.pop();
110+
if (Visited.find(BB) != Visited.end())
111+
continue;
112+
Visited.insert(BB);
113+
114+
for (const MCInst &Inst : *BB) {
115+
// Two loads one after another - skip whole function
116+
if (BC.MIB->isAArch64ExclusiveLoad(Inst) && IsLoad) {
117+
if (opts::Verbosity >= 2) {
118+
outs() << "BOLT-INSTRUMENTER: function " << Function.getPrintName()
119+
<< " has two exclusive loads. Ignoring the function.\n";
120+
}
101121
return true;
102122
}
103123

104-
return false;
105-
}
124+
if (BC.MIB->isAArch64ExclusiveLoad(Inst))
125+
IsLoad = true;
126+
127+
if (IsLoad && BBToSkip.find(BB) == BBToSkip.end()) {
128+
BBToSkip.insert(BB);
129+
if (opts::Verbosity >= 2) {
130+
outs() << "BOLT-INSTRUMENTER: skip BB " << BB->getName()
131+
<< " due to exclusive instruction in function "
132+
<< Function.getPrintName() << "\n";
133+
}
134+
}
135+
136+
if (!IsLoad && BC.MIB->isAArch64ExclusiveStore(Inst)) {
137+
if (opts::Verbosity >= 2) {
138+
outs() << "BOLT-INSTRUMENTER: function " << Function.getPrintName()
139+
<< " has exclusive store without corresponding load. Ignoring "
140+
"the function.\n";
141+
}
142+
return true;
143+
}
144+
145+
if (IsLoad && (BC.MIB->isAArch64ExclusiveStore(Inst) ||
146+
BC.MIB->isAArch64ExclusiveClear(Inst)))
147+
IsLoad = false;
148+
}
106149

107-
uint32_t Instrumentation::getFunctionNameIndex(const BinaryFunction &Function) {
108-
auto Iter = FuncToStringIdx.find(&Function);
109-
if (Iter != FuncToStringIdx.end())
110-
return Iter->second;
111-
size_t Idx = Summary->StringTable.size();
112-
FuncToStringIdx.emplace(std::make_pair(&Function, Idx));
113-
Summary->StringTable.append(getEscapedName(Function.getOneName()));
114-
Summary->StringTable.append(1, '\0');
115-
return Idx;
150+
if (IsLoad && BB->succ_size() == 0) {
151+
if (opts::Verbosity >= 2) {
152+
outs()
153+
<< "BOLT-INSTRUMENTER: function " << Function.getPrintName()
154+
<< " has exclusive load in trailing BB. Ignoring the function.\n";
155+
}
156+
return true;
157+
}
158+
159+
for (BinaryBasicBlock *BBS : BB->successors())
160+
BBQueue.push({BBS, IsLoad});
161+
}
162+
163+
if (BBToSkip.size() == Visited.size()) {
164+
if (opts::Verbosity >= 2) {
165+
outs() << "BOLT-INSTRUMENTER: all BBs are marked with true. Ignoring the "
166+
"function "
167+
<< Function.getPrintName() << "\n";
168+
}
169+
return true;
170+
}
171+
172+
return false;
116173
}
117174

118175
bool Instrumentation::createCallDescription(FunctionDescription &FuncDesc,
@@ -307,7 +364,8 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function,
307364
if (BC.isMachO() && Function.hasName("___GLOBAL_init_65535/1"))
308365
return;
309366

310-
if (BC.isAArch64() && hasAArch64ExclusiveMemop(Function))
367+
std::unordered_set<const BinaryBasicBlock *> BBToSkip;
368+
if (BC.isAArch64() && hasAArch64ExclusiveMemop(Function, BBToSkip))
311369
return;
312370

313371
SplitWorklistTy SplitWorklist;
@@ -389,6 +447,11 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function,
389447

390448
for (auto BBI = Function.begin(), BBE = Function.end(); BBI != BBE; ++BBI) {
391449
BinaryBasicBlock &BB = *BBI;
450+
451+
// Skip BBs with exclusive load/stores
452+
if (BBToSkip.find(&BB) != BBToSkip.end())
453+
continue;
454+
392455
bool HasUnconditionalBranch = false;
393456
bool HasJumpTable = false;
394457
bool IsInvokeBlock = InvokeBlocks.count(&BB) > 0;

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -270,32 +270,38 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
270270
return isLDRB(Inst) || isLDRH(Inst) || isLDRW(Inst) || isLDRX(Inst);
271271
}
272272

273-
bool isAArch64Exclusive(const MCInst &Inst) const override {
273+
bool isAArch64ExclusiveLoad(const MCInst &Inst) const override {
274274
return (Inst.getOpcode() == AArch64::LDXPX ||
275275
Inst.getOpcode() == AArch64::LDXPW ||
276276
Inst.getOpcode() == AArch64::LDXRX ||
277277
Inst.getOpcode() == AArch64::LDXRW ||
278278
Inst.getOpcode() == AArch64::LDXRH ||
279279
Inst.getOpcode() == AArch64::LDXRB ||
280-
Inst.getOpcode() == AArch64::STXPX ||
281-
Inst.getOpcode() == AArch64::STXPW ||
282-
Inst.getOpcode() == AArch64::STXRX ||
283-
Inst.getOpcode() == AArch64::STXRW ||
284-
Inst.getOpcode() == AArch64::STXRH ||
285-
Inst.getOpcode() == AArch64::STXRB ||
286280
Inst.getOpcode() == AArch64::LDAXPX ||
287281
Inst.getOpcode() == AArch64::LDAXPW ||
288282
Inst.getOpcode() == AArch64::LDAXRX ||
289283
Inst.getOpcode() == AArch64::LDAXRW ||
290284
Inst.getOpcode() == AArch64::LDAXRH ||
291-
Inst.getOpcode() == AArch64::LDAXRB ||
285+
Inst.getOpcode() == AArch64::LDAXRB);
286+
}
287+
288+
bool isAArch64ExclusiveStore(const MCInst &Inst) const override {
289+
return (Inst.getOpcode() == AArch64::STXPX ||
290+
Inst.getOpcode() == AArch64::STXPW ||
291+
Inst.getOpcode() == AArch64::STXRX ||
292+
Inst.getOpcode() == AArch64::STXRW ||
293+
Inst.getOpcode() == AArch64::STXRH ||
294+
Inst.getOpcode() == AArch64::STXRB ||
292295
Inst.getOpcode() == AArch64::STLXPX ||
293296
Inst.getOpcode() == AArch64::STLXPW ||
294297
Inst.getOpcode() == AArch64::STLXRX ||
295298
Inst.getOpcode() == AArch64::STLXRW ||
296299
Inst.getOpcode() == AArch64::STLXRH ||
297-
Inst.getOpcode() == AArch64::STLXRB ||
298-
Inst.getOpcode() == AArch64::CLREX);
300+
Inst.getOpcode() == AArch64::STLXRB);
301+
}
302+
303+
bool isAArch64ExclusiveClear(const MCInst &Inst) const override {
304+
return (Inst.getOpcode() == AArch64::CLREX);
299305
}
300306

301307
bool isLoadFromStack(const MCInst &Inst) const {

‎bolt/test/AArch64/exclusive-instrument.s

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,108 @@
66
// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
77
// RUN: %s -o %t.o
88
// RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe -nostdlib -Wl,-q -Wl,-fini=dummy
9-
// RUN: llvm-bolt %t.exe -o %t.bolt -instrument -v=1 | FileCheck %s
9+
// RUN: llvm-bolt %t.exe -o %t.bolt -instrument -v=2 | FileCheck %s
1010

11-
// CHECK: Function foo has exclusive instructions, skip instrumentation
11+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function foo
12+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function foo
13+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function foo
14+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function case1
15+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function case2
16+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function case2
17+
// CHECK: BOLT-INSTRUMENTER: function case3 has exclusive store without corresponding load. Ignoring the function.
18+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function case4
19+
// CHECK: BOLT-INSTRUMENTER: function case4 has two exclusive loads. Ignoring the function.
20+
// CHECK: BOLT-INSTRUMENTER: skip BB {{.*}} due to exclusive instruction in function case5
21+
// CHECK: BOLT-INSTRUMENTER: function case5 has exclusive load in trailing BB. Ignoring the function.
1222

1323
.global foo
1424
.type foo, %function
1525
foo:
26+
# exclusive load and store in two bbs
1627
ldaxr w9, [x10]
1728
cbnz w9, .Lret
1829
stlxr w12, w11, [x9]
1930
cbz w12, foo
20-
clrex
2131
.Lret:
32+
clrex
2233
ret
2334
.size foo, .-foo
2435

2536
.global _start
2637
.type _start, %function
2738
_start:
28-
cmp x0, #0
29-
b.eq .Lexit
30-
bl foo
31-
.Lexit:
39+
mov x0, #0
40+
mov x1, #1
41+
mov x2, #2
42+
mov x3, #3
43+
44+
bl case1
45+
bl case2
46+
bl case3
47+
bl case4
48+
bl case5
49+
3250
ret
3351
.size _start, .-_start
3452

53+
# Case 1: exclusive load and store in one basic block
54+
.global case1
55+
.type case1, %function
56+
case1:
57+
str x0, [x2]
58+
ldxr w0, [x2]
59+
add w0, w0, #1
60+
stxr w1, w0, [x2]
61+
ret
62+
.size case1, .-case1
63+
64+
# Case 2: exclusive load and store in different blocks
65+
.global case2
66+
.type case2, %function
67+
case2:
68+
b case2_load
69+
70+
case2_load:
71+
ldxr x0, [x2]
72+
b case2_store
73+
74+
case2_store:
75+
add x0, x0, #1
76+
stxr w1, x0, [x2]
77+
ret
78+
.size case2, .-case2
79+
80+
# Case 3: store without preceding load
81+
.global case3
82+
.type case3, %function
83+
case3:
84+
stxr w1, x3, [x2]
85+
ret
86+
.size case3, .-case3
87+
88+
# Case 4: two exclusive load instructions in neighboring blocks
89+
.global case4
90+
.type case4, %function
91+
case4:
92+
b case4_load
93+
94+
case4_load:
95+
ldxr x0, [x2]
96+
b case4_load_next
97+
98+
case4_load_next:
99+
ldxr x1, [x2]
100+
ret
101+
.size case4, .-case4
102+
103+
# Case 5: Exclusive load without successor
104+
.global case5
105+
.type case5, %function
106+
case5:
107+
ldxr x0, [x2]
108+
ret
109+
.size case5, .-case5
110+
35111
.global dummy
36112
.type dummy, %function
37113
dummy:

0 commit comments

Comments
 (0)