[llvm-commits] [llvm] r168037 - in /llvm/trunk: include/llvm/Constant.h lib/Transforms/IPO/GlobalOpt.cpp lib/VMCore/Constants.cpp test/Transforms/GlobalOpt/tls.ll

[Hans Wennborg]

Author: hans
Date: Thu Nov 15 05:40:00 2012
New Revision: 168037

URL: http://llvm.org/viewvc/llvm-project?rev=168037&view=rev
Log:
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.

Added:
    llvm/trunk/test/Transforms/GlobalOpt/tls.ll
Modified:
    llvm/trunk/include/llvm/Constant.h
    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
    llvm/trunk/lib/VMCore/Constants.cpp

Modified: llvm/trunk/include/llvm/Constant.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Constant.h?rev=168037&r1=168036&r2=168037&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Constant.h (original)
+++ llvm/trunk/include/llvm/Constant.h Thu Nov 15 05:40:00 2012
@@ -65,6 +65,9 @@
   /// 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;

Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=168037&r1=168036&r2=168037&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Thu Nov 15 05:40:00 2012
@@ -225,6 +225,7 @@
 
         // 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 @@
           if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(
                                                            SI->getOperand(1))) {
             Value *StoredVal = SI->getOperand(0);
+
+            if (Constant *C = dyn_cast<Constant>(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;

Modified: llvm/trunk/lib/VMCore/Constants.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Constants.cpp?rev=168037&r1=168036&r2=168037&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/Constants.cpp (original)
+++ llvm/trunk/lib/VMCore/Constants.cpp Thu Nov 15 05:40:00 2012
@@ -245,6 +245,31 @@
   }
 }
 
+/// isThreadDependent - Return true if the value can vary between threads.
+bool Constant::isThreadDependent() const {
+  SmallPtrSet<const Constant*, 64> Visited;
+  SmallVector<const Constant*, 64> WorkList;
+  WorkList.push_back(this);
+  Visited.insert(this);
+
+  while (!WorkList.empty()) {
+    const Constant *C = WorkList.pop_back_val();
+
+    if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(C)) {
+      if (GV->isThreadLocal())
+        return true;
+    }
+
+    for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) {
+      const Constant *D = cast<Constant>(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 {

Added: llvm/trunk/test/Transforms/GlobalOpt/tls.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/tls.ll?rev=168037&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GlobalOpt/tls.ll (added)
+++ llvm/trunk/test/Transforms/GlobalOpt/tls.ll Thu Nov 15 05:40:00 2012
@@ -0,0 +1,53 @@
+; RUN: opt < %s -globalopt -S | FileCheck %s
+
+declare void @wait()
+declare void @signal()
+declare void @start_thread(void ()*)
+
+ at x = internal thread_local global [100 x i32] zeroinitializer, align 16
+ at 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
+}