Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 1839858

Browse files
committed
Make GlobalOpt be conservative with TLS variables (PR14309)
For global variables that get the same value stored into them everywhere, GlobalOpt will replace them with a constant. The problem is that a thread-local GlobalVariable looks like one value (the address of the TLS var), but is different between threads. This patch introduces Constant::isThreadDependent() which returns true for thread-local variables and constants which depend on them (e.g. a GEP into a thread-local array), and teaches GlobalOpt not to track such values. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@168037 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent ac39a03 commit 1839858

File tree

4 files changed

+90
-0
lines changed

4 files changed

+90
-0
lines changed

include/llvm/Constant.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class Constant : public User {
6565
/// true for things like constant expressions that could divide by zero.
6666
bool canTrap() const;
6767

68+
/// isThreadDependent - Return true if the value can vary between threads.
69+
bool isThreadDependent() const;
70+
6871
/// isConstantUsed - Return true if the constant has users other than constant
6972
/// exprs and other dangling things.
7073
bool isConstantUsed() const;

lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ static bool AnalyzeGlobal(const Value *V, GlobalStatus &GS,
225225

226226
// Don't hack on volatile stores.
227227
if (SI->isVolatile()) return true;
228+
228229
GS.Ordering = StrongerOrdering(GS.Ordering, SI->getOrdering());
229230

230231
// If this is a direct store to the global (i.e., the global is a scalar
@@ -234,6 +235,14 @@ static bool AnalyzeGlobal(const Value *V, GlobalStatus &GS,
234235
if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(
235236
SI->getOperand(1))) {
236237
Value *StoredVal = SI->getOperand(0);
238+
239+
if (Constant *C = dyn_cast<Constant>(StoredVal)) {
240+
if (C->isThreadDependent()) {
241+
// The stored value changes between threads; don't track it.
242+
return true;
243+
}
244+
}
245+
237246
if (StoredVal == GV->getInitializer()) {
238247
if (GS.StoredType < GlobalStatus::isInitializerStored)
239248
GS.StoredType = GlobalStatus::isInitializerStored;

lib/VMCore/Constants.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,31 @@ bool Constant::canTrap() const {
245245
}
246246
}
247247

248+
/// isThreadDependent - Return true if the value can vary between threads.
249+
bool Constant::isThreadDependent() const {
250+
SmallPtrSet<const Constant*, 64> Visited;
251+
SmallVector<const Constant*, 64> WorkList;
252+
WorkList.push_back(this);
253+
Visited.insert(this);
254+
255+
while (!WorkList.empty()) {
256+
const Constant *C = WorkList.pop_back_val();
257+
258+
if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(C)) {
259+
if (GV->isThreadLocal())
260+
return true;
261+
}
262+
263+
for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) {
264+
const Constant *D = cast<Constant>(C->getOperand(I));
265+
if (Visited.insert(D))
266+
WorkList.push_back(D);
267+
}
268+
}
269+
270+
return false;
271+
}
272+
248273
/// isConstantUsed - Return true if the constant has users other than constant
249274
/// exprs and other dangling things.
250275
bool Constant::isConstantUsed() const {

test/Transforms/GlobalOpt/tls.ll

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
; RUN: opt < %s -globalopt -S | FileCheck %s
2+
3+
declare void @wait()
4+
declare void @signal()
5+
declare void @start_thread(void ()*)
6+
7+
@x = internal thread_local global [100 x i32] zeroinitializer, align 16
8+
@ip = internal global i32* null, align 8
9+
10+
; PR14309: GlobalOpt would think that the value of @ip is always the address of
11+
; x[1]. However, that address is different for different threads so @ip cannot
12+
; be replaced with a constant.
13+
14+
define i32 @f() {
15+
entry:
16+
; Set @ip to point to x[1] for thread 1.
17+
store i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), i32** @ip, align 8
18+
19+
; Run g on a new thread.
20+
tail call void @start_thread(void ()* @g) nounwind
21+
tail call void @wait() nounwind
22+
23+
; Reset x[1] for thread 1.
24+
store i32 0, i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), align 4
25+
26+
; Read the value of @ip, which now points at x[1] for thread 2.
27+
%0 = load i32** @ip, align 8
28+
29+
%1 = load i32* %0, align 4
30+
ret i32 %1
31+
32+
; CHECK: @f
33+
; Make sure that the load from @ip hasn't been removed.
34+
; CHECK: load i32** @ip
35+
; CHECK: ret
36+
}
37+
38+
define internal void @g() nounwind uwtable {
39+
entry:
40+
; Set @ip to point to x[1] for thread 2.
41+
store i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), i32** @ip, align 8
42+
43+
; Store 50 in x[1] for thread 2.
44+
store i32 50, i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), align 4
45+
46+
tail call void @signal() nounwind
47+
ret void
48+
49+
; CHECK: @g
50+
; Make sure that the store to @ip hasn't been removed.
51+
; CHECK: store {{.*}} @ip
52+
; CHECK: ret
53+
}

0 commit comments

Comments
 (0)