Make succ_iterator a real random access iterator and clean up a couple of users.

llvm-svn: 201088

include/llvm/Support/CFG.h

@@ -101,23 +101,45 @@ inline const_pred_iterator pred_end(const BasicBlock *BB) {
 //===----------------------------------------------------------------------===//
 
 template <class Term_, class BB_>           // Successor Iterator
-class SuccIterator : public std::iterator<std::bidirectional_iterator_tag,
-                                          BB_, ptrdiff_t, BB_*, BB_*> {
+class SuccIterator : public std::iterator<std::random_access_iterator_tag, BB_,
+                                          int, BB_ *, BB_ *> {
+  typedef std::iterator<std::random_access_iterator_tag, BB_, int, BB_ *, BB_ *>
+  super;
+
+public:
+  typedef typename super::pointer pointer;
+  typedef typename super::reference reference;
+
+private:
   const Term_ Term;
   unsigned idx;
-  typedef std::iterator<std::bidirectional_iterator_tag, BB_, ptrdiff_t, BB_*,
-                                                                    BB_*> super;
   typedef SuccIterator<Term_, BB_> Self;
 
   inline bool index_is_valid(int idx) {
     return idx >= 0 && (unsigned) idx < Term->getNumSuccessors();
   }
 
-public:
-  typedef typename super::pointer pointer;
-  typedef typename super::reference reference;
-  // TODO: This can be random access iterator, only operator[] missing.
+  /// \brief Proxy object to allow write access in operator[]
+  class SuccessorProxy {
+    Self it;
 
+  public:
+    explicit SuccessorProxy(const Self &it) : it(it) {}
+
+    SuccessorProxy &operator=(SuccessorProxy r) {
+      *this = reference(r);
+      return *this;
+    }
+
+    SuccessorProxy &operator=(reference r) {
+      it.Term->setSuccessor(it.idx, r);
+      return *this;
+    }
+
+    operator reference() const { return *it; }
+  };
+
+public:
   explicit inline SuccIterator(Term_ T) : Term(T), idx(0) {// begin iterator
   }
   inline SuccIterator(Term_ T, bool)                       // end iterator
@@ -206,15 +228,11 @@ public:
     return distance;
   }
 
-  // This works for read access, however write access is difficult as changes
-  // to Term are only possible with Term->setSuccessor(idx). Pointers that can
-  // be modified are not available.
-  //
-  // inline pointer operator[](int offset) {
-  //  Self tmp = *this;
-  //  tmp += offset;
-  //  return tmp.operator*();
-  // }
+  inline SuccessorProxy operator[](int offset) {
+   Self tmp = *this;
+   tmp += offset;
+   return SuccessorProxy(tmp);
+  }
 
   /// Get the source BB of this iterator.
   inline BB_ *getSource() {

lib/Analysis/CFG.cpp

@@ -168,8 +168,7 @@ static bool isPotentiallyReachableInner(SmallVectorImpl<BasicBlock *> &Worklist,
       // ignoring any other blocks inside the loop body.
       Outer->getExitBlocks(Worklist);
     } else {
-      for (succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I)
-        Worklist.push_back(*I);
+      Worklist.append(succ_begin(BB), succ_end(BB));
     }
   } while (!Worklist.empty());
 
@@ -222,8 +221,7 @@ bool llvm::isPotentiallyReachable(const Instruction *A, const Instruction *B,
       return false;
 
     // Otherwise, continue doing the normal per-BB CFG walk.
-    for (succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I)
-      Worklist.push_back(*I);
+    Worklist.append(succ_begin(BB), succ_end(BB));
 
     if (Worklist.empty()) {
       // We've proven that there's no path!

lib/Analysis/TargetTransformInfo.cpp

@@ -437,12 +437,7 @@ struct NoTTI LLVM_FINAL : ImmutablePass, TargetTransformInfo {
         return TopTTI->getCallCost(cast<FunctionType>(FTy), CS.arg_size());
       }
 
-      SmallVector<const Value *, 8> Arguments;
-      for (ImmutableCallSite::arg_iterator AI = CS.arg_begin(),
-                                           AE = CS.arg_end();
-           AI != AE; ++AI)
-        Arguments.push_back(*AI);
-
+      SmallVector<const Value *, 8> Arguments(CS.arg_begin(), CS.arg_end());
       return TopTTI->getCallCost(F, Arguments);
     }
 

lib/Transforms/Scalar/GVN.cpp

@@ -817,8 +817,7 @@ SpeculationFailure:
     // Mark as unavailable.
     EntryVal = 0;
 
-    for (succ_iterator I = succ_begin(Entry), E = succ_end(Entry); I != E; ++I)
-      BBWorklist.push_back(*I);
+    BBWorklist.append(succ_begin(Entry), succ_end(Entry));
   } while (!BBWorklist.empty());
 
   return false;

unittests/Analysis/CFGTest.cpp

@@ -115,7 +115,7 @@ protected:
     PM.add(P);
     PM.run(*M);
   }
-private:
+
   OwningPtr<Module> M;
   Instruction *A, *B;
 };
@@ -352,27 +352,40 @@ TEST_F(IsPotentiallyReachableTest, OneLoopAfterTheOtherInsideAThirdLoop) {
   ExpectPath(true);
 }
 
+static const char *BranchInsideLoopIR =
+    "declare i1 @switch()\n"
+    "\n"
+    "define void @test() {\n"
+    "entry:\n"
+    "  br label %loop\n"
+    "loop:\n"
+    "  %x = call i1 @switch()\n"
+    "  br i1 %x, label %nextloopblock, label %exit\n"
+    "nextloopblock:\n"
+    "  %y = call i1 @switch()\n"
+    "  br i1 %y, label %left, label %right\n"
+    "left:\n"
+    "  %A = bitcast i8 undef to i8\n"
+    "  br label %loop\n"
+    "right:\n"
+    "  %B = bitcast i8 undef to i8\n"
+    "  br label %loop\n"
+    "exit:\n"
+    "  ret void\n"
+    "}";
+
 TEST_F(IsPotentiallyReachableTest, BranchInsideLoop) {
-  ParseAssembly(
-      "declare i1 @switch()\n"
-      "\n"
-      "define void @test() {\n"
-      "entry:\n"
-      "  br label %loop\n"
-      "loop:\n"
-      "  %x = call i1 @switch()\n"
-      "  br i1 %x, label %nextloopblock, label %exit\n"
-      "nextloopblock:\n"
-      "  %y = call i1 @switch()\n"
-      "  br i1 %y, label %left, label %right\n"
-      "left:\n"
-      "  %A = bitcast i8 undef to i8\n"
-      "  br label %loop\n"
-      "right:\n"
-      "  %B = bitcast i8 undef to i8\n"
-      "  br label %loop\n"
-      "exit:\n"
-      "  ret void\n"
-      "}");
+  ParseAssembly(BranchInsideLoopIR);
+  ExpectPath(true);
+}
+
+TEST_F(IsPotentiallyReachableTest, ModifyTest) {
+  ParseAssembly(BranchInsideLoopIR);
+
+  succ_iterator S = succ_begin(++M->getFunction("test")->begin());
+  BasicBlock *OldBB = S[0];
+  S[0] = S[1];
+  ExpectPath(false);
+  S[0] = OldBB;
   ExpectPath(true);
 }