From bbd301d97a8440d1cb08a64372f23de677ce05e8 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Mon, 11 May 2015 22:20:48 +0000 Subject: [PATCH] Readdress r236990, use of static members on a non-static variable. The TargetRegistry is just a namespace-like class, instantiated in one place to use a range-based for loop. Instead, expose access to the registry via a range-based 'targets()' function instead. This makes most uses a bit awkward/more verbose - but eventually we should just add a range-based find_if function which will streamline these functions. I'm happy to mkae them a bit awkward in the interim as encouragement to improve the algorithms in time. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@237059 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/TargetRegistry.h | 13 ++++--- lib/ExecutionEngine/TargetSelect.cpp | 14 ++++---- lib/Support/TargetRegistry.cpp | 52 ++++++++++++--------------- lib/Target/TargetMachineC.cpp | 15 ++++---- unittests/Support/TargetRegistry.cpp | 5 ++- 5 files changed, 46 insertions(+), 53 deletions(-) diff --git a/include/llvm/Support/TargetRegistry.h b/include/llvm/Support/TargetRegistry.h index a4bf51f67f3..afcdbb85f85 100644 --- a/include/llvm/Support/TargetRegistry.h +++ b/include/llvm/Support/TargetRegistry.h @@ -543,7 +543,14 @@ namespace llvm { /// TargetRegistry - Generic interface to target specific features. struct TargetRegistry { - class iterator { + // FIXME: Make this a namespace, probably just move all the Register* + // functions into Target (currently they all just set members on the Target + // anyway, and Target friends this class so those functions can... + // function). + TargetRegistry() = delete; + + class iterator + : public std::iterator { const Target *Current; explicit iterator(Target *T) : Current(T) {} friend struct TargetRegistry; @@ -586,9 +593,7 @@ namespace llvm { /// @name Registry Access /// @{ - static iterator begin(); - - static iterator end() { return iterator(); } + static iterator_range targets(); /// lookupTarget - Lookup a target based on a target triple. /// diff --git a/lib/ExecutionEngine/TargetSelect.cpp b/lib/ExecutionEngine/TargetSelect.cpp index e6679cfb7f7..57f6e080414 100644 --- a/lib/ExecutionEngine/TargetSelect.cpp +++ b/lib/ExecutionEngine/TargetSelect.cpp @@ -49,21 +49,19 @@ TargetMachine *EngineBuilder::selectTarget(const Triple &TargetTriple, // Adjust the triple to match what the user requested. const Target *TheTarget = nullptr; if (!MArch.empty()) { - for (TargetRegistry::iterator it = TargetRegistry::begin(), - ie = TargetRegistry::end(); it != ie; ++it) { - if (MArch == it->getName()) { - TheTarget = &*it; - break; - } - } + auto I = std::find_if( + TargetRegistry::targets().begin(), TargetRegistry::targets().end(), + [&](const Target &T) { return MArch == T.getName(); }); - if (!TheTarget) { + if (I == TargetRegistry::targets().end()) { if (ErrorStr) *ErrorStr = "No available targets are compatible with this -march, " "see -version for the available targets.\n"; return nullptr; } + TheTarget = &*I; + // Adjust the triple to match (if known), otherwise stick with the // requested/host triple. Triple::ArchType Type = Triple::getArchTypeForLLVMName(MArch); diff --git a/lib/Support/TargetRegistry.cpp b/lib/Support/TargetRegistry.cpp index 3ca85728985..eefef8ad8ea 100644 --- a/lib/Support/TargetRegistry.cpp +++ b/lib/Support/TargetRegistry.cpp @@ -18,8 +18,8 @@ using namespace llvm; // Clients are responsible for avoid race conditions in registration. static Target *FirstTarget = nullptr; -TargetRegistry::iterator TargetRegistry::begin() { - return iterator(FirstTarget); +iterator_range TargetRegistry::targets() { + return make_range(iterator(FirstTarget), iterator()); } const Target *TargetRegistry::lookupTarget(const std::string &ArchName, @@ -30,19 +30,17 @@ const Target *TargetRegistry::lookupTarget(const std::string &ArchName, // name, because it might be a backend that has no mapping to a target triple. const Target *TheTarget = nullptr; if (!ArchName.empty()) { - for (TargetRegistry::iterator it = TargetRegistry::begin(), - ie = TargetRegistry::end(); it != ie; ++it) { - if (ArchName == it->getName()) { - TheTarget = &*it; - break; - } - } + auto I = + std::find_if(targets().begin(), targets().end(), + [&](const Target &T) { return ArchName == T.getName(); }); - if (!TheTarget) { + if (I == targets().end()) { Error = "error: invalid target '" + ArchName + "'.\n"; return nullptr; } + TheTarget = &*I; + // Adjust the triple to match (if known), otherwise stick with the // given triple. Triple::ArchType Type = Triple::getArchTypeForLLVMName(ArchName); @@ -66,30 +64,28 @@ const Target *TargetRegistry::lookupTarget(const std::string &ArchName, const Target *TargetRegistry::lookupTarget(const std::string &TT, std::string &Error) { // Provide special warning when no targets are initialized. - if (begin() == end()) { + if (targets().begin() == targets().end()) { Error = "Unable to find target for this triple (no targets are registered)"; return nullptr; } - const Target *Matching = nullptr; - Triple::ArchType Arch = Triple(TT).getArch(); - for (iterator it = begin(), ie = end(); it != ie; ++it) { - if (it->ArchMatchFn(Arch)) { - if (Matching) { - Error = std::string("Cannot choose between targets \"") + - Matching->Name + "\" and \"" + it->Name + "\""; - return nullptr; - } - Matching = &*it; - } - } + Triple::ArchType Arch = Triple(TT).getArch(); + auto ArchMatch = [&](const Target &T) { return T.ArchMatchFn(Arch); }; + auto I = std::find_if(targets().begin(), targets().end(), ArchMatch); - if (!Matching) { + if (I == targets().end()) { Error = "No available targets are compatible with this triple, " "see -version for the available targets."; return nullptr; } - return Matching; + auto J = std::find_if(std::next(I), targets().end(), ArchMatch); + if (J != targets().end()) { + Error = std::string("Cannot choose between targets \"") + I->Name + + "\" and \"" + J->Name + "\""; + return nullptr; + } + + return &*I; } void TargetRegistry::RegisterTarget(Target &T, @@ -123,10 +119,8 @@ static int TargetArraySortFn(const std::pair *LHS, void TargetRegistry::printRegisteredTargetsForVersion() { std::vector > Targets; size_t Width = 0; - for (TargetRegistry::iterator I = TargetRegistry::begin(), - E = TargetRegistry::end(); - I != E; ++I) { - Targets.push_back(std::make_pair(I->getName(), &*I)); + for (const auto &T : TargetRegistry::targets()) { + Targets.push_back(std::make_pair(T.getName(), &T)); Width = std::max(Width, Targets.back().first.size()); } array_pod_sort(Targets.begin(), Targets.end(), TargetArraySortFn); diff --git a/lib/Target/TargetMachineC.cpp b/lib/Target/TargetMachineC.cpp index 1a5bf163bd9..623b3e8ca32 100644 --- a/lib/Target/TargetMachineC.cpp +++ b/lib/Target/TargetMachineC.cpp @@ -47,11 +47,11 @@ inline LLVMTargetRef wrap(const Target * P) { } LLVMTargetRef LLVMGetFirstTarget() { - if(TargetRegistry::begin() == TargetRegistry::end()) { + if (TargetRegistry::targets().begin() == TargetRegistry::targets().end()) { return nullptr; } - const Target* target = &*TargetRegistry::begin(); + const Target *target = &*TargetRegistry::targets().begin(); return wrap(target); } LLVMTargetRef LLVMGetNextTarget(LLVMTargetRef T) { @@ -60,13 +60,10 @@ LLVMTargetRef LLVMGetNextTarget(LLVMTargetRef T) { LLVMTargetRef LLVMGetTargetFromName(const char *Name) { StringRef NameRef = Name; - for (TargetRegistry::iterator IT = TargetRegistry::begin(), - IE = TargetRegistry::end(); IT != IE; ++IT) { - if (IT->getName() == NameRef) - return wrap(&*IT); - } - - return nullptr; + auto I = std::find_if( + TargetRegistry::targets().begin(), TargetRegistry::targets().end(), + [&](const Target &T) { return T.getName() == NameRef; }); + return I != TargetRegistry::targets().end() ? wrap(&*I) : nullptr; } LLVMBool LLVMGetTargetFromTriple(const char* TripleStr, LLVMTargetRef *T, diff --git a/unittests/Support/TargetRegistry.cpp b/unittests/Support/TargetRegistry.cpp index 2dd57fa7bf1..ae89c8b6493 100644 --- a/unittests/Support/TargetRegistry.cpp +++ b/unittests/Support/TargetRegistry.cpp @@ -22,9 +22,8 @@ TEST(TargetRegistry, TargetHasArchType) { llvm::InitializeAllTargetInfos(); - for (auto I = TargetRegistry::begin(), E = TargetRegistry::end(); - I != E; ++I) { - StringRef Name = I->getName(); + for (const Target &T : TargetRegistry::targets()) { + StringRef Name = T.getName(); // There is really no way (at present) to ask a Target whether it targets // a specific architecture, because the logic for that is buried in a // predicate. -- 2.34.1