From 160dcf5b61b8d328cd1705a90c1e0ae27dcabd41 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Fri, 20 Jun 2014 00:38:12 +0000 Subject: [PATCH] Don't build switch lookup tables for dllimport or TLS variables We would previously put dllimport variables in switch lookup tables, which doesn't work because the address cannot be used in a constant initializer. This is basically the same problem that we have in PR19955. Putting TLS variables in switch tables also desn't work, because the address of such a variable is not constant. Differential Revision: http://reviews.llvm.org/D4220 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@211331 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Constant.h | 3 ++ lib/IR/Constants.cpp | 53 ++++++++++++------- lib/Transforms/Utils/SimplifyCFG.cpp | 4 ++ .../SimplifyCFG/X86/switch_to_lookup_table.ll | 52 ++++++++++++++++++ 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/include/llvm/IR/Constant.h b/include/llvm/IR/Constant.h index 39c7c37dafd..257bb80f211 100644 --- a/include/llvm/IR/Constant.h +++ b/include/llvm/IR/Constant.h @@ -71,6 +71,9 @@ public: /// isThreadDependent - Return true if the value can vary between threads. bool isThreadDependent() const; + /// Return true if the value is dependent on a dllimport variable. + bool isDLLImportDependent() const; + /// isConstantUsed - Return true if the constant has users other than constant /// exprs and other dangling things. bool isConstantUsed() const; diff --git a/lib/IR/Constants.cpp b/lib/IR/Constants.cpp index aa26cff6a7b..5851625383b 100644 --- a/lib/IR/Constants.cpp +++ b/lib/IR/Constants.cpp @@ -278,35 +278,48 @@ bool Constant::canTrap() const { return canTrapImpl(this, NonTrappingOps); } -/// 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); +/// Check if C contains a GlobalValue for which Predicate is true. +static bool +ConstHasGlobalValuePredicate(const Constant *C, + bool (*Predicate)(const GlobalValue *)) { + SmallPtrSet Visited; + SmallVector WorkList; + WorkList.push_back(C); + Visited.insert(C); while (!WorkList.empty()) { - const Constant *C = WorkList.pop_back_val(); - - if (const GlobalVariable *GV = dyn_cast(C)) { - if (GV->isThreadLocal()) + const Constant *WorkItem = WorkList.pop_back_val(); + if (const auto *GV = dyn_cast(WorkItem)) + if (Predicate(GV)) return true; - } - - for (unsigned I = 0, E = C->getNumOperands(); I != E; ++I) { - const Constant *D = dyn_cast(C->getOperand(I)); - if (!D) + for (const Value *Op : WorkItem->operands()) { + const Constant *ConstOp = dyn_cast(Op); + if (!ConstOp) continue; - if (Visited.insert(D)) - WorkList.push_back(D); + if (Visited.insert(ConstOp)) + WorkList.push_back(ConstOp); } } - return false; } -/// isConstantUsed - Return true if the constant has users other than constant -/// exprs and other dangling things. +/// Return true if the value can vary between threads. +bool Constant::isThreadDependent() const { + auto DLLImportPredicate = [](const GlobalValue *GV) { + return GV->isThreadLocal(); + }; + return ConstHasGlobalValuePredicate(this, DLLImportPredicate); +} + +bool Constant::isDLLImportDependent() const { + auto DLLImportPredicate = [](const GlobalValue *GV) { + return GV->hasDLLImportStorageClass(); + }; + return ConstHasGlobalValuePredicate(this, DLLImportPredicate); +} + +/// Return true if the constant has users other than constant exprs and other +/// dangling things. bool Constant::isConstantUsed() const { for (const User *U : users()) { const Constant *UC = dyn_cast(U); diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp index e155daf6fcc..ff2f2a03622 100644 --- a/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/lib/Transforms/Utils/SimplifyCFG.cpp @@ -3313,6 +3313,10 @@ static bool ForwardSwitchConditionToPHI(SwitchInst *SI) { static bool ValidLookupTableConstant(Constant *C) { if (ConstantExpr *CE = dyn_cast(C)) return CE->isGEPWithNoNotionalOverIndexing(); + if (C->isThreadDependent()) + return false; + if (C->isDLLImportDependent()) + return false; return isa(C) || isa(C) || diff --git a/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll b/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll index 81079b1aa5a..ee63d2c0c0f 100644 --- a/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll +++ b/test/Transforms/SimplifyCFG/X86/switch_to_lookup_table.ll @@ -918,3 +918,55 @@ return: ; CHECK: switch i32 ; CHECK-NOT: @switch.table } + +; Don't build tables for switches with TLS variables. +@tls_a = thread_local global i32 0 +@tls_b = thread_local global i32 0 +@tls_c = thread_local global i32 0 +@tls_d = thread_local global i32 0 +define i32* @tls(i32 %x) { +entry: + switch i32 %x, label %sw.default [ + i32 0, label %return + i32 1, label %sw.bb1 + i32 2, label %sw.bb2 + ] +sw.bb1: + br label %return +sw.bb2: + br label %return +sw.default: + br label %return +return: + %retval.0 = phi i32* [ @tls_d, %sw.default ], [ @tls_c, %sw.bb2 ], [ @tls_b, %sw.bb1 ], [ @tls_a, %entry ] + ret i32* %retval.0 +; CHECK-LABEL: @tls( +; CHECK: switch i32 +; CHECK-NOT: @switch.table +} + +; Don't build tables for switches with dllimport variables. +@dllimport_a = external dllimport global i32 +@dllimport_b = external dllimport global i32 +@dllimport_c = external dllimport global i32 +@dllimport_d = external dllimport global i32 +define i32* @dllimport(i32 %x) { +entry: + switch i32 %x, label %sw.default [ + i32 0, label %return + i32 1, label %sw.bb1 + i32 2, label %sw.bb2 + ] +sw.bb1: + br label %return +sw.bb2: + br label %return +sw.default: + br label %return +return: + %retval.0 = phi i32* [ @dllimport_d, %sw.default ], [ @dllimport_c, %sw.bb2 ], [ @dllimport_b, %sw.bb1 ], [ @dllimport_a, %entry ] + ret i32* %retval.0 +; CHECK-LABEL: @dllimport( +; CHECK: switch i32 +; CHECK-NOT: @switch.table +} -- 2.34.1