From 21863332ca402263c518840742aee2b126d53007 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Fri, 16 Nov 2012 20:14:40 +0000 Subject: Merge r168037 from trunk: 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/branches/release_32@168192 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Constant.h | 3 +++ lib/Transforms/IPO/GlobalOpt.cpp | 9 +++++++ lib/VMCore/Constants.cpp | 25 +++++++++++++++++++ test/Transforms/GlobalOpt/tls.ll | 53 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+) create mode 100644 test/Transforms/GlobalOpt/tls.ll diff --git a/include/llvm/Constant.h b/include/llvm/Constant.h index 7fecf4c7b4..0ddd1db6c0 100644 --- a/include/llvm/Constant.h +++ b/include/llvm/Constant.h @@ -65,6 +65,9 @@ public: /// true for things like constant expressions that could divide by zero. bool canTrap() const; + /// isThreadDependent - Return true if the value can vary between threads. + bool isThreadDependent() const; + /// isConstantUsed - Return true if the constant has users other than constant /// exprs and other dangling things. bool isConstantUsed() const; diff --git a/lib/Transforms/IPO/GlobalOpt.cpp b/lib/Transforms/IPO/GlobalOpt.cpp index 678189b3d6..591278fa62 100644 --- a/lib/Transforms/IPO/GlobalOpt.cpp +++ b/lib/Transforms/IPO/GlobalOpt.cpp @@ -225,6 +225,7 @@ static bool AnalyzeGlobal(const Value *V, GlobalStatus &GS, // Don't hack on volatile stores. if (SI->isVolatile()) return true; + GS.Ordering = StrongerOrdering(GS.Ordering, SI->getOrdering()); // 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, if (const GlobalVariable *GV = dyn_cast( SI->getOperand(1))) { Value *StoredVal = SI->getOperand(0); + + if (Constant *C = dyn_cast(StoredVal)) { + if (C->isThreadDependent()) { + // The stored value changes between threads; don't track it. + return true; + } + } + if (StoredVal == GV->getInitializer()) { if (GS.StoredType < GlobalStatus::isInitializerStored) GS.StoredType = GlobalStatus::isInitializerStored; diff --git a/lib/VMCore/Constants.cpp b/lib/VMCore/Constants.cpp index a4e21e16b3..041fe2eb4f 100644 --- a/lib/VMCore/Constants.cpp +++ b/lib/VMCore/Constants.cpp @@ -245,6 +245,31 @@ bool Constant::canTrap() const { } } +/// isThreadDependent - Return true if the value can vary between threads. +bool Constant::isThreadDependent() const { + SmallPtrSet Visited; + SmallVector WorkList; + WorkList.push_back(this); + Visited.insert(this); + + while (!WorkList.empty()) { + const Constant *C = WorkList.pop_back_val(); + + if (const GlobalVariable *GV = dyn_cast(C)) { + if (GV->isThreadLocal()) + return true; + } + + for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) { + const Constant *D = cast(C->getOperand(I)); + if (Visited.insert(D)) + WorkList.push_back(D); + } + } + + return false; +} + /// isConstantUsed - Return true if the constant has users other than constant /// exprs and other dangling things. bool Constant::isConstantUsed() const { diff --git a/test/Transforms/GlobalOpt/tls.ll b/test/Transforms/GlobalOpt/tls.ll new file mode 100644 index 0000000000..7a410e5ed2 --- /dev/null +++ b/test/Transforms/GlobalOpt/tls.ll @@ -0,0 +1,53 @@ +; RUN: opt < %s -globalopt -S | FileCheck %s + +declare void @wait() +declare void @signal() +declare void @start_thread(void ()*) + +@x = internal thread_local global [100 x i32] zeroinitializer, align 16 +@ip = internal global i32* null, align 8 + +; PR14309: GlobalOpt would think that the value of @ip is always the address of +; x[1]. However, that address is different for different threads so @ip cannot +; be replaced with a constant. + +define i32 @f() { +entry: + ; Set @ip to point to x[1] for thread 1. + store i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), i32** @ip, align 8 + + ; Run g on a new thread. + tail call void @start_thread(void ()* @g) nounwind + tail call void @wait() nounwind + + ; Reset x[1] for thread 1. + store i32 0, i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), align 4 + + ; Read the value of @ip, which now points at x[1] for thread 2. + %0 = load i32** @ip, align 8 + + %1 = load i32* %0, align 4 + ret i32 %1 + +; CHECK: @f +; Make sure that the load from @ip hasn't been removed. +; CHECK: load i32** @ip +; CHECK: ret +} + +define internal void @g() nounwind uwtable { +entry: + ; Set @ip to point to x[1] for thread 2. + store i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), i32** @ip, align 8 + + ; Store 50 in x[1] for thread 2. + store i32 50, i32* getelementptr inbounds ([100 x i32]* @x, i64 0, i64 1), align 4 + + tail call void @signal() nounwind + ret void + +; CHECK: @g +; Make sure that the store to @ip hasn't been removed. +; CHECK: store {{.*}} @ip +; CHECK: ret +} -- cgit v1.2.3