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