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.

llvm-svn: 237059

llvm/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<std::forward_iterator_tag, Target, ptrdiff_t> {
       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<iterator> targets();
 
     /// lookupTarget - Lookup a target based on a target triple.
     ///

llvm/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);

llvm/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::iterator> 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<StringRef, const Target *> *LHS,
 void TargetRegistry::printRegisteredTargetsForVersion() {
   std::vector<std::pair<StringRef, const Target*> > 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);

llvm/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,

llvm/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.