[llvm] r276001 - [RegionInfo] Some cleanups

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 27 11:34:01 PDT 2016


Fixed in r276901.

On Wed, Jul 27, 2016 at 1:02 PM, Reid Kleckner <rnk at google.com> wrote:

> I think this broke the build with libstdc++ 4.7, which this modules bot
> appears to use:
>
> http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules-2/builds/3250/steps/compile/logs/stdio
>
> On Tue, Jul 19, 2016 at 10:50 AM, David Majnemer via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: majnemer
>> Date: Tue Jul 19 12:50:30 2016
>> New Revision: 276001
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276001&view=rev
>> Log:
>> [RegionInfo] Some cleanups
>>
>> - Use unique_ptr instead of managing a container of new'd pointers.
>> - Use range based for loops.
>>
>> No functional change is intended.
>>
>> Modified:
>>     llvm/trunk/include/llvm/Analysis/RegionInfo.h
>>     llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h
>>
>> Modified: llvm/trunk/include/llvm/Analysis/RegionInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfo.h?rev=276001&r1=276000&r2=276001&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/RegionInfo.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/RegionInfo.h Tue Jul 19 12:50:30 2016
>> @@ -39,6 +39,7 @@
>>
>>  #include "llvm/ADT/DepthFirstIterator.h"
>>  #include "llvm/ADT/PointerIntPair.h"
>> +#include "llvm/ADT/iterator_range.h"
>>  #include "llvm/IR/CFG.h"
>>  #include "llvm/IR/Dominators.h"
>>  #include "llvm/IR/PassManager.h"
>> @@ -278,7 +279,7 @@ class RegionBase : public RegionNodeBase
>>    // The subregions of this region.
>>    RegionSet children;
>>
>> -  typedef std::map<BlockT *, RegionNodeT *> BBNodeMapT;
>> +  typedef std::map<BlockT *, std::unique_ptr<RegionNodeT>> BBNodeMapT;
>>
>>    // Save the BasicBlock RegionNodes that are element of this Region.
>>    mutable BBNodeMapT BBNodeMap;
>> @@ -634,9 +635,15 @@ public:
>>
>>    element_iterator element_begin();
>>    element_iterator element_end();
>> +  iterator_range<element_iterator> elements() {
>> +    return make_range(element_begin(), element_end());
>> +  }
>>
>>    const_element_iterator element_begin() const;
>>    const_element_iterator element_end() const;
>> +  iterator_range<const_element_iterator> elements() const {
>> +    return make_range(element_begin(), element_end());
>> +  }
>>    //@}
>>  };
>>
>>
>> Modified: llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h?rev=276001&r1=276000&r2=276001&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/RegionInfoImpl.h Tue Jul 19 12:50:30
>> 2016
>> @@ -38,12 +38,6 @@ RegionBase<Tr>::RegionBase(BlockT *Entry
>>
>>  template <class Tr>
>>  RegionBase<Tr>::~RegionBase() {
>> -  // Free the cached nodes.
>> -  for (typename BBNodeMapT::iterator it = BBNodeMap.begin(),
>> -                                     ie = BBNodeMap.end();
>> -       it != ie; ++it)
>> -    delete it->second;
>> -
>>    // Only clean the cache for this Region. Caches of child Regions will
>> be
>>    // cleaned when the child Regions are deleted.
>>    BBNodeMap.clear();
>> @@ -71,10 +65,9 @@ void RegionBase<Tr>::replaceEntryRecursi
>>      RegionQueue.pop_back();
>>
>>      R->replaceEntry(NewEntry);
>> -    for (typename RegionT::const_iterator RI = R->begin(), RE = R->end();
>> -         RI != RE; ++RI) {
>> -      if ((*RI)->getEntry() == OldEntry)
>> -        RegionQueue.push_back(RI->get());
>> +    for (std::unique_ptr<RegionT> &Child : *R) {
>> +      if (Child->getEntry() == OldEntry)
>> +        RegionQueue.push_back(Child.get());
>>      }
>>    }
>>  }
>> @@ -90,10 +83,9 @@ void RegionBase<Tr>::replaceExitRecursiv
>>      RegionQueue.pop_back();
>>
>>      R->replaceExit(NewExit);
>> -    for (typename RegionT::const_iterator RI = R->begin(), RE = R->end();
>> -         RI != RE; ++RI) {
>> -      if ((*RI)->getExit() == OldExit)
>> -        RegionQueue.push_back(RI->get());
>> +    for (std::unique_ptr<RegionT> &Child : *R) {
>> +      if (Child->getExit() == OldExit)
>> +        RegionQueue.push_back(Child.get());
>>      }
>>    }
>>  }
>> @@ -160,13 +152,10 @@ typename Tr::LoopT *RegionBase<Tr>::oute
>>  template <class Tr>
>>  typename RegionBase<Tr>::BlockT *RegionBase<Tr>::getEnteringBlock()
>> const {
>>    BlockT *entry = getEntry();
>> -  BlockT *Pred;
>>    BlockT *enteringBlock = nullptr;
>>
>> -  for (PredIterTy PI = InvBlockTraits::child_begin(entry),
>> -                  PE = InvBlockTraits::child_end(entry);
>> -       PI != PE; ++PI) {
>> -    Pred = *PI;
>> +  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(entry),
>> +                                 InvBlockTraits::child_end(entry))) {
>>      if (DT->getNode(Pred) && !contains(Pred)) {
>>        if (enteringBlock)
>>          return nullptr;
>> @@ -181,16 +170,13 @@ typename RegionBase<Tr>::BlockT *RegionB
>>  template <class Tr>
>>  typename RegionBase<Tr>::BlockT *RegionBase<Tr>::getExitingBlock() const
>> {
>>    BlockT *exit = getExit();
>> -  BlockT *Pred;
>>    BlockT *exitingBlock = nullptr;
>>
>>    if (!exit)
>>      return nullptr;
>>
>> -  for (PredIterTy PI = InvBlockTraits::child_begin(exit),
>> -                  PE = InvBlockTraits::child_end(exit);
>> -       PI != PE; ++PI) {
>> -    Pred = *PI;
>> +  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(exit),
>> +                                 InvBlockTraits::child_end(exit))) {
>>      if (contains(Pred)) {
>>        if (exitingBlock)
>>          return nullptr;
>> @@ -239,19 +225,17 @@ void RegionBase<Tr>::verifyBBInRegion(Bl
>>
>>    BlockT *entry = getEntry(), *exit = getExit();
>>
>> -  for (SuccIterTy SI = BlockTraits::child_begin(BB),
>> -                  SE = BlockTraits::child_end(BB);
>> -       SI != SE; ++SI) {
>> -    if (!contains(*SI) && exit != *SI)
>> +  for (BlockT *Succ :
>> +       make_range(BlockTraits::child_begin(BB),
>> BlockTraits::child_end(BB))) {
>> +    if (!contains(Succ) && exit != Succ)
>>        llvm_unreachable("Broken region found: edges leaving the region
>> must go "
>>                         "to the exit node!");
>>    }
>>
>>    if (entry != BB) {
>> -    for (PredIterTy SI = InvBlockTraits::child_begin(BB),
>> -                    SE = InvBlockTraits::child_end(BB);
>> -         SI != SE; ++SI) {
>> -      if (!contains(*SI))
>> +    for (BlockT *Pred : make_range(InvBlockTraits::child_begin(BB),
>> +                                   InvBlockTraits::child_end(BB))) {
>> +      if (!contains(Pred))
>>          llvm_unreachable("Broken region found: edges entering the region
>> must "
>>                           "go to the entry node!");
>>      }
>> @@ -266,11 +250,10 @@ void RegionBase<Tr>::verifyWalk(BlockT *
>>
>>    verifyBBInRegion(BB);
>>
>> -  for (SuccIterTy SI = BlockTraits::child_begin(BB),
>> -                  SE = BlockTraits::child_end(BB);
>> -       SI != SE; ++SI) {
>> -    if (*SI != exit && visited->find(*SI) == visited->end())
>> -      verifyWalk(*SI, visited);
>> +  for (BlockT *Succ :
>> +       make_range(BlockTraits::child_begin(BB),
>> BlockTraits::child_end(BB))) {
>> +    if (Succ != exit && visited->find(Succ) == visited->end())
>> +      verifyWalk(Succ, visited);
>>    }
>>  }
>>
>> @@ -288,9 +271,8 @@ void RegionBase<Tr>::verifyRegion() cons
>>
>>  template <class Tr>
>>  void RegionBase<Tr>::verifyRegionNest() const {
>> -  for (typename RegionT::const_iterator RI = begin(), RE = end(); RI !=
>> RE;
>> -       ++RI)
>> -    (*RI)->verifyRegionNest();
>> +  for (const std::unique_ptr<RegionT> &R : *this)
>> +    R->verifyRegionNest();
>>
>>    verifyRegion();
>>  }
>> @@ -345,13 +327,14 @@ typename Tr::RegionNodeT *RegionBase<Tr>
>>
>>    typename BBNodeMapT::const_iterator at = BBNodeMap.find(BB);
>>
>> -  if (at != BBNodeMap.end())
>> -    return at->second;
>> -
>> -  auto Deconst = const_cast<RegionBase<Tr> *>(this);
>> -  RegionNodeT *NewNode = new RegionNodeT(static_cast<RegionT
>> *>(Deconst), BB);
>> -  BBNodeMap.insert(std::make_pair(BB, NewNode));
>> -  return NewNode;
>> +  if (at == BBNodeMap.end()) {
>> +    auto Deconst = const_cast<RegionBase<Tr> *>(this);
>> +    at = BBNodeMap
>> +             .emplace(BB, make_unique<RegionNodeT>(
>> +                              static_cast<RegionT *>(Deconst), BB))
>> +             .first;
>> +  }
>> +  return at->second.get();
>>  }
>>
>>  template <class Tr>
>> @@ -365,9 +348,9 @@ typename Tr::RegionNodeT *RegionBase<Tr>
>>
>>  template <class Tr>
>>  void RegionBase<Tr>::transferChildrenTo(RegionT *To) {
>> -  for (iterator I = begin(), E = end(); I != E; ++I) {
>> -    (*I)->parent = To;
>> -    To->children.push_back(std::move(*I));
>> +  for (std::unique_ptr<RegionT> &R : *this) {
>> +    R->parent = To;
>> +    To->children.push_back(std::move(R));
>>    }
>>    children.clear();
>>  }
>> @@ -375,9 +358,10 @@ void RegionBase<Tr>::transferChildrenTo(
>>  template <class Tr>
>>  void RegionBase<Tr>::addSubRegion(RegionT *SubRegion, bool moveChildren)
>> {
>>    assert(!SubRegion->parent && "SubRegion already has a parent!");
>> -  assert(std::find_if(begin(), end(), [&](const std::unique_ptr<RegionT>
>> &R) {
>> -           return R.get() == SubRegion;
>> -         }) == children.end() &&
>> +  assert(find_if(*this,
>> +                 [&](const std::unique_ptr<RegionT> &R) {
>> +                   return R.get() == SubRegion;
>> +                 }) == children.end() &&
>>           "Subregion already exists!");
>>
>>    SubRegion->parent = static_cast<RegionT *>(this);
>> @@ -389,9 +373,9 @@ void RegionBase<Tr>::addSubRegion(Region
>>    assert(SubRegion->children.empty() &&
>>           "SubRegions that contain children are not supported");
>>
>> -  for (element_iterator I = element_begin(), E = element_end(); I != E;
>> ++I) {
>> -    if (!(*I)->isSubRegion()) {
>> -      BlockT *BB = (*I)->template getNodeAs<BlockT>();
>> +  for (RegionNodeT *Element : elements()) {
>> +    if (!Element->isSubRegion()) {
>> +      BlockT *BB = Element->template getNodeAs<BlockT>();
>>
>>        if (SubRegion->contains(BB))
>>          RI->setRegionFor(BB, SubRegion);
>> @@ -399,12 +383,12 @@ void RegionBase<Tr>::addSubRegion(Region
>>    }
>>
>>    std::vector<std::unique_ptr<RegionT>> Keep;
>> -  for (iterator I = begin(), E = end(); I != E; ++I) {
>> -    if (SubRegion->contains(I->get()) && I->get() != SubRegion) {
>> -      (*I)->parent = SubRegion;
>> -      SubRegion->children.push_back(std::move(*I));
>> +  for (std::unique_ptr<RegionT> &R : *this) {
>> +    if (SubRegion->contains(R.get()) && R.get() != SubRegion) {
>> +      R->parent = SubRegion;
>> +      SubRegion->children.push_back(std::move(R));
>>      } else
>> -      Keep.push_back(std::move(*I));
>> +      Keep.push_back(std::move(R));
>>    }
>>
>>    children.clear();
>> @@ -418,9 +402,10 @@ template <class Tr>
>>  typename Tr::RegionT *RegionBase<Tr>::removeSubRegion(RegionT *Child) {
>>    assert(Child->parent == this && "Child is not a child of this
>> region!");
>>    Child->parent = nullptr;
>> -  typename RegionSet::iterator I = std::find_if(
>> -      children.begin(), children.end(),
>> -      [&](const std::unique_ptr<RegionT> &R) { return R.get() == Child;
>> });
>> +  typename RegionSet::iterator I =
>> +      find_if(children, [&](const std::unique_ptr<RegionT> &R) {
>> +        return R.get() == Child;
>> +      });
>>    assert(I != children.end() && "Region does not exit. Unable to
>> remove.");
>>    children.erase(children.begin() + (I - begin()));
>>    return Child;
>> @@ -446,10 +431,9 @@ typename Tr::RegionT *RegionBase<Tr>::ge
>>    RegionT *R = RI->getRegionFor(exit);
>>
>>    if (R->getEntry() != exit) {
>> -    for (PredIterTy PI = InvBlockTraits::child_begin(getExit()),
>> -                    PE = InvBlockTraits::child_end(getExit());
>> -         PI != PE; ++PI)
>> -      if (!contains(*PI))
>> +    for (BlockT *Pred :
>> make_range(InvBlockTraits::child_begin(getExit()),
>> +                                   InvBlockTraits::child_end(getExit())))
>> +      if (!contains(Pred))
>>          return nullptr;
>>      if (Tr::getNumSuccessors(exit) == 1)
>>        return new RegionT(getEntry(), *BlockTraits::child_begin(exit),
>> RI, DT);
>> @@ -459,10 +443,9 @@ typename Tr::RegionT *RegionBase<Tr>::ge
>>    while (R->getParent() && R->getParent()->getEntry() == exit)
>>      R = R->getParent();
>>
>> -  for (PredIterTy PI = InvBlockTraits::child_begin(getExit()),
>> -                  PE = InvBlockTraits::child_end(getExit());
>> -       PI != PE; ++PI) {
>> -    if (!(contains(*PI) || R->contains(*PI)))
>> +  for (BlockT *Pred : make_range(InvBlockTraits::child_begin(getExit()),
>> +                                 InvBlockTraits::child_end(getExit()))) {
>> +    if (!(contains(Pred) || R->contains(Pred)))
>>        return nullptr;
>>    }
>>
>> @@ -487,9 +470,8 @@ void RegionBase<Tr>::print(raw_ostream &
>>        for (const auto *BB : blocks())
>>          OS << BB->getName() << ", "; // TODO: remove the last ","
>>      } else if (Style == PrintRN) {
>> -      for (const_element_iterator I = element_begin(), E = element_end();
>> -           I != E; ++I) {
>> -        OS << **I << ", "; // TODO: remove the last ",
>> +      for (const RegionNodeT *Element : elements()) {
>> +        OS << *Element << ", "; // TODO: remove the last ",
>>        }
>>      }
>>
>> @@ -497,8 +479,8 @@ void RegionBase<Tr>::print(raw_ostream &
>>    }
>>
>>    if (print_tree) {
>> -    for (const_iterator RI = begin(), RE = end(); RI != RE; ++RI)
>> -      (*RI)->print(OS, print_tree, level + 1, Style);
>> +    for (const std::unique_ptr<RegionT> &R : *this)
>> +      R->print(OS, print_tree, level + 1, Style);
>>    }
>>
>>    if (Style != PrintNone)
>> @@ -514,15 +496,9 @@ void RegionBase<Tr>::dump() const {
>>
>>  template <class Tr>
>>  void RegionBase<Tr>::clearNodeCache() {
>> -  // Free the cached nodes.
>> -  for (typename BBNodeMapT::iterator I = BBNodeMap.begin(),
>> -                                     IE = BBNodeMap.end();
>> -       I != IE; ++I)
>> -    delete I->second;
>> -
>>    BBNodeMap.clear();
>> -  for (typename RegionT::iterator RI = begin(), RE = end(); RI != RE;
>> ++RI)
>> -    (*RI)->clearNodeCache();
>> +  for (std::unique_ptr<RegionT> &R : *this)
>> +    R->clearNodeCache();
>>  }
>>
>>
>>  //===----------------------------------------------------------------------===//
>> @@ -541,12 +517,12 @@ RegionInfoBase<Tr>::~RegionInfoBase() {
>>  template <class Tr>
>>  void RegionInfoBase<Tr>::verifyBBMap(const RegionT *R) const {
>>    assert(R && "Re must be non-null");
>> -  for (auto I = R->element_begin(), E = R->element_end(); I != E; ++I) {
>> -    if (I->isSubRegion()) {
>> -      const RegionT *SR = I->template getNodeAs<RegionT>();
>> +  for (const typename Tr::RegionNodeT *Element : R->elements()) {
>> +    if (Element->isSubRegion()) {
>> +      const RegionT *SR = Element->template getNodeAs<RegionT>();
>>        verifyBBMap(SR);
>>      } else {
>> -      BlockT *BB = I->template getNodeAs<BlockT>();
>> +      BlockT *BB = Element->template getNodeAs<BlockT>();
>>        if (getRegionFor(BB) != R)
>>          llvm_unreachable("BB map does not match region nesting");
>>      }
>> @@ -556,10 +532,8 @@ void RegionInfoBase<Tr>::verifyBBMap(con
>>  template <class Tr>
>>  bool RegionInfoBase<Tr>::isCommonDomFrontier(BlockT *BB, BlockT *entry,
>>                                               BlockT *exit) const {
>> -  for (PredIterTy PI = InvBlockTraits::child_begin(BB),
>> -                  PE = InvBlockTraits::child_end(BB);
>> -       PI != PE; ++PI) {
>> -    BlockT *P = *PI;
>> +  for (BlockT *P : make_range(InvBlockTraits::child_begin(BB),
>> +                              InvBlockTraits::child_end(BB))) {
>>      if (DT->dominates(entry, P) && !DT->dominates(exit, P))
>>        return false;
>>    }
>> @@ -590,20 +564,18 @@ bool RegionInfoBase<Tr>::isRegion(BlockT
>>    DST *exitSuccs = &DF->find(exit)->second;
>>
>>    // Do not allow edges leaving the region.
>> -  for (typename DST::iterator SI = entrySuccs->begin(), SE =
>> entrySuccs->end();
>> -       SI != SE; ++SI) {
>> -    if (*SI == exit || *SI == entry)
>> +  for (BlockT *Succ : *entrySuccs) {
>> +    if (Succ == exit || Succ == entry)
>>        continue;
>> -    if (exitSuccs->find(*SI) == exitSuccs->end())
>> +    if (exitSuccs->find(Succ) == exitSuccs->end())
>>        return false;
>> -    if (!isCommonDomFrontier(*SI, entry, exit))
>> +    if (!isCommonDomFrontier(Succ, entry, exit))
>>        return false;
>>    }
>>
>>    // Do not allow edges pointing into the region.
>> -  for (typename DST::iterator SI = exitSuccs->begin(), SE =
>> exitSuccs->end();
>> -       SI != SE; ++SI) {
>> -    if (DT->properlyDominates(entry, *SI) && *SI != exit)
>> +  for (BlockT *Succ : *exitSuccs) {
>> +    if (DT->properlyDominates(entry, Succ) && Succ != exit)
>>        return false;
>>    }
>>
>> @@ -663,7 +635,7 @@ typename Tr::RegionT *RegionInfoBase<Tr>
>>
>>    RegionT *region =
>>        new RegionT(entry, exit, static_cast<RegionInfoT *>(this), DT);
>> -  BBtoRegion.insert(std::make_pair(entry, region));
>> +  BBtoRegion.insert({entry, region});
>>
>>  #ifdef EXPENSIVE_CHECKS
>>    region->verifyRegion();
>> @@ -758,9 +730,8 @@ void RegionInfoBase<Tr>::buildRegionsTre
>>      BBtoRegion[BB] = region;
>>    }
>>
>> -  for (typename DomTreeNodeT::iterator CI = N->begin(), CE = N->end();
>> CI != CE;
>> -       ++CI) {
>> -    buildRegionsTree(*CI, region);
>> +  for (DomTreeNodeBase<BlockT> *C : *N) {
>> +    buildRegionsTree(C, region);
>>    }
>>  }
>>
>> @@ -850,10 +821,9 @@ RegionInfoBase<Tr>::getMaxRegionExit(Blo
>>             ExitR->getParent()->getEntry() == Exit)
>>        ExitR = ExitR->getParent();
>>
>> -    for (PredIterTy PI = InvBlockTraits::child_begin(Exit),
>> -                    PE = InvBlockTraits::child_end(Exit);
>> -         PI != PE; ++PI) {
>> -      if (!R->contains(*PI) && !ExitR->contains(*PI))
>> +    for (BlockT *Pred : make_range(InvBlockTraits::child_begin(Exit),
>> +                                   InvBlockTraits::child_end(Exit))) {
>> +      if (!R->contains(Pred) && !ExitR->contains(Pred))
>>          break;
>>      }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160727/5af62977/attachment.html>


More information about the llvm-commits mailing list