[polly] r238090 - Use unique_ptr to clarify ownership of ScopStmt

David Blaikie dblaikie at gmail.com
Tue May 26 11:00:13 PDT 2015


On Fri, May 22, 2015 at 10:14 PM, Tobias Grosser <tobias at grosser.es> wrote:

> Author: grosser
> Date: Sat May 23 00:14:09 2015
> New Revision: 238090
>
> URL: http://llvm.org/viewvc/llvm-project?rev=238090&view=rev
> Log:
> Use unique_ptr to clarify ownership of ScopStmt


> Modified:
>     polly/trunk/include/polly/ScopInfo.h
>     polly/trunk/lib/Analysis/DependenceInfo.cpp
>     polly/trunk/lib/Analysis/ScopInfo.cpp
>     polly/trunk/lib/CodeGen/IRBuilder.cpp
>     polly/trunk/lib/CodeGen/Utils.cpp
>     polly/trunk/lib/Exchange/JSONExporter.cpp
>     polly/trunk/lib/Transform/ScheduleOptimizer.cpp
>
> Modified: polly/trunk/include/polly/ScopInfo.h
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopInfo.h?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/include/polly/ScopInfo.h (original)
> +++ polly/trunk/include/polly/ScopInfo.h Sat May 23 00:14:09 2015
> @@ -766,7 +766,7 @@ private:
>    /// Max loop depth.
>    unsigned MaxLoopDepth;
>
> -  typedef std::vector<ScopStmt *> StmtSet;
> +  typedef std::vector<std::unique_ptr<ScopStmt>> StmtSet;
>    /// The statements in this Scop.
>    StmtSet Stmts;
>
>
> Modified: polly/trunk/lib/Analysis/DependenceInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/DependenceInfo.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/Analysis/DependenceInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/DependenceInfo.cpp Sat May 23 00:14:09 2015
> @@ -79,12 +79,12 @@ static void collectInfo(Scop &S, isl_uni
>    *StmtSchedule = isl_union_map_empty(Space);
>
>    SmallPtrSet<const Value *, 8> ReductionBaseValues;
> -  for (ScopStmt *Stmt : S)
> +  for (auto &Stmt : S)
>      for (MemoryAccess *MA : *Stmt)
>        if (MA->isReductionLike())
>          ReductionBaseValues.insert(MA->getBaseAddr());
>
> -  for (ScopStmt *Stmt : S) {
> +  for (auto &Stmt : S) {
>      for (MemoryAccess *MA : *Stmt) {
>        isl_set *domcp = Stmt->getDomain();
>        isl_map *accdom = MA->getAccessRelation();
> @@ -361,7 +361,7 @@ void Dependences::calculateDependences(S
>
>    // Step 1)
>    RED = isl_union_map_empty(isl_union_map_get_space(RAW));
> -  for (ScopStmt *Stmt : S) {
> +  for (auto &Stmt : S) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isReductionLike())
>          continue;
> @@ -404,7 +404,7 @@ void Dependences::calculateDependences(S
>    // We then move this portion of reduction dependences back to the
> statement ->
>    // statement space and add a mapping from the memory access to these
>    // dependences.
> -  for (ScopStmt *Stmt : S) {
> +  for (auto &Stmt : S) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isReductionLike())
>          continue;
> @@ -476,13 +476,13 @@ bool Dependences::isValidSchedule(Scop &
>
>    isl_space *ScheduleSpace = nullptr;
>
> -  for (ScopStmt *Stmt : S) {
> +  for (auto &Stmt : S) {
>      isl_map *StmtScat;
>
> -    if (NewSchedule->find(Stmt) == NewSchedule->end())
> +    if (NewSchedule->find(&*Stmt) == NewSchedule->end())
>        StmtScat = Stmt->getSchedule();
>      else
> -      StmtScat = isl_map_copy((*NewSchedule)[Stmt]);
> +      StmtScat = isl_map_copy((*NewSchedule)[&*Stmt]);
>
>      if (!ScheduleSpace)
>        ScheduleSpace = isl_space_range(isl_map_get_space(StmtScat));
>
> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Sat May 23 00:14:09 2015
> @@ -1349,7 +1349,7 @@ void Scop::realignParams() {
>    // Align the parameters of all data structures to the model.
>    Context = isl_set_align_params(Context, Space);
>
> -  for (ScopStmt *Stmt : *this)
> +  for (auto &Stmt : *this)
>      Stmt->realignParams();
>  }
>
> @@ -1476,7 +1476,7 @@ bool Scop::buildAliasGroups(AliasAnalysi
>
>    DenseMap<Value *, MemoryAccess *> PtrToAcc;
>    DenseSet<Value *> HasWriteAccess;
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>
>      // Skip statements with an empty domain as they will never be
> executed.
>      isl_set *StmtDomain = Stmt->getDomain();
> @@ -1666,7 +1666,7 @@ void Scop::dropConstantScheduleDims() {
>      isl_val_free(FixedVal);
>    }
>
> -  for (auto *S : *this) {
> +  for (auto &S : *this) {
>      isl_map *Schedule = S->getSchedule();
>      Schedule = isl_map_apply_range(Schedule, isl_map_copy(DropDimMap));
>      S->setSchedule(Schedule);
> @@ -1704,10 +1704,6 @@ Scop::~Scop() {
>    isl_set_free(Context);
>    isl_set_free(AssumedContext);
>
> -  // Free the statements;
> -  for (ScopStmt *Stmt : *this)
> -    delete Stmt;
> -
>    // Free the ScopArrayInfo objects.
>    for (auto &ScopArrayInfoPair : arrays())
>      delete ScopArrayInfoPair.second;
> @@ -1817,7 +1813,7 @@ void Scop::printAliasAssumptions(raw_ost
>  void Scop::printStatements(raw_ostream &OS) const {
>    OS << "Statements {\n";
>
> -  for (ScopStmt *Stmt : *this)
> +  for (auto &Stmt : *this)
>      OS.indent(4) << *Stmt;
>
>    OS.indent(4) << "}\n";
> @@ -1850,7 +1846,7 @@ isl_ctx *Scop::getIslCtx() const { retur
>  __isl_give isl_union_set *Scop::getDomains() {
>    isl_union_set *Domain = isl_union_set_empty(getParamSpace());
>
> -  for (ScopStmt *Stmt : *this)
> +  for (auto &Stmt : *this)
>      Domain = isl_union_set_add_set(Domain, Stmt->getDomain());
>
>    return Domain;
> @@ -1859,7 +1855,7 @@ __isl_give isl_union_set *Scop::getDomai
>  __isl_give isl_union_map *Scop::getMustWrites() {
>    isl_union_map *Write = isl_union_map_empty(this->getParamSpace());
>
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isMustWrite())
>          continue;
> @@ -1876,7 +1872,7 @@ __isl_give isl_union_map *Scop::getMustW
>  __isl_give isl_union_map *Scop::getMayWrites() {
>    isl_union_map *Write = isl_union_map_empty(this->getParamSpace());
>
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isMayWrite())
>          continue;
> @@ -1893,7 +1889,7 @@ __isl_give isl_union_map *Scop::getMayWr
>  __isl_give isl_union_map *Scop::getWrites() {
>    isl_union_map *Write = isl_union_map_empty(this->getParamSpace());
>
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isWrite())
>          continue;
> @@ -1910,7 +1906,7 @@ __isl_give isl_union_map *Scop::getWrite
>  __isl_give isl_union_map *Scop::getReads() {
>    isl_union_map *Read = isl_union_map_empty(getParamSpace());
>
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>      for (MemoryAccess *MA : *Stmt) {
>        if (!MA->isRead())
>          continue;
> @@ -1928,7 +1924,7 @@ __isl_give isl_union_map *Scop::getReads
>  __isl_give isl_union_map *Scop::getSchedule() {
>    isl_union_map *Schedule = isl_union_map_empty(getParamSpace());
>
> -  for (ScopStmt *Stmt : *this)
> +  for (auto &Stmt : *this)
>      Schedule = isl_union_map_add_map(Schedule, Stmt->getSchedule());
>
>    return isl_union_map_coalesce(Schedule);
> @@ -1936,7 +1932,7 @@ __isl_give isl_union_map *Scop::getSched
>
>  bool Scop::restrictDomains(__isl_take isl_union_set *Domain) {
>    bool Changed = false;
> -  for (ScopStmt *Stmt : *this) {
> +  for (auto &Stmt : *this) {
>      isl_union_set *StmtDomain = isl_union_set_from_set(Stmt->getDomain());
>      isl_union_set *NewStmtDomain = isl_union_set_intersect(
>          isl_union_set_copy(StmtDomain), isl_union_set_copy(Domain));
> @@ -1975,22 +1971,23 @@ void Scop::addScopStmt(BasicBlock *BB, R
>                         const Region &CurRegion,
>                         SmallVectorImpl<Loop *> &NestLoops,
>                         SmallVectorImpl<unsigned> &ScheduleVec) {
> -  ScopStmt *Stmt;
> +  std::unique_ptr<ScopStmt> Stmt;
>
>    if (BB) {
> -    Stmt =
> -        new ScopStmt(*this, tempScop, CurRegion, *BB, NestLoops,
> ScheduleVec);
> -    StmtMap[BB] = Stmt;
> +    Stmt = std::unique_ptr<ScopStmt>(
> +        new ScopStmt(*this, tempScop, CurRegion, *BB, NestLoops,
> ScheduleVec));
> +    StmtMap[BB] = &*Stmt;
>    } else {
>      assert(R && "Either a basic block or a region is needed to "
>                  "create a new SCoP stmt.");
> -    Stmt = new ScopStmt(*this, tempScop, CurRegion, *R, NestLoops,
> ScheduleVec);
> +    Stmt = std::unique_ptr<ScopStmt>(
> +        new ScopStmt(*this, tempScop, CurRegion, *R, NestLoops,
> ScheduleVec));
>      for (BasicBlock *BB : R->blocks())
> -      StmtMap[BB] = Stmt;
> +      StmtMap[BB] = &*Stmt;
>    }
>
>    // Insert all statements into the statement map and the statement
> vector.
> -  Stmts.push_back(Stmt);
> +  Stmts.push_back(std::move(Stmt));
>

It looks like these objects aren't polymorphic (it's not like Stmts
contains pointers to heterogenous derived objects). If that's the case, you
might want to consider whether you can use value rather than pointer
semantics.

Either a std::vector<ScopStmt> or, if you need object pointer identity, a
std::forward_list<ScopStmt> or std::list<ScopStmt>

This makes the semantics even simpler - no extra pointer chasing, etc, and
can reduce the memory footprint (forward_list reduces the memory footprint
a tiny bit (compared to a vector of unique_ptr) but std::list actually
increases it because it needs two pointers per node instead of one (a
vector of unique_ptrs only has one pointer per node, etc))


>
>    // Increasing the Schedule function is OK for the moment, because
>    // we are using a depth first iterator and the program is well
> structured.
>
> Modified: polly/trunk/lib/CodeGen/IRBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/IRBuilder.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/CodeGen/IRBuilder.cpp (original)
> +++ polly/trunk/lib/CodeGen/IRBuilder.cpp Sat May 23 00:14:09 2015
> @@ -59,7 +59,7 @@ void ScopAnnotator::buildAliasScopes(Sco
>    OtherAliasScopeListMap.clear();
>
>    SetVector<Value *> BasePtrs;
> -  for (ScopStmt *Stmt : S)
> +  for (auto &Stmt : S)
>      for (MemoryAccess *MA : *Stmt)
>        BasePtrs.insert(MA->getBaseAddr());
>
>
> Modified: polly/trunk/lib/CodeGen/Utils.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/Utils.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/CodeGen/Utils.cpp (original)
> +++ polly/trunk/lib/CodeGen/Utils.cpp Sat May 23 00:14:09 2015
> @@ -37,7 +37,7 @@ BasicBlock *polly::executeScopConditiona
>      std::string OldName = OldBlock->getName();
>
>      // Update ScopInfo.
> -    for (ScopStmt *Stmt : S)
> +    for (auto &Stmt : S)
>        if (Stmt->getBasicBlock() == OldBlock) {
>          Stmt->setBasicBlock(NewBlock);
>          break;
>
> Modified: polly/trunk/lib/Exchange/JSONExporter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Exchange/JSONExporter.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/Exchange/JSONExporter.cpp (original)
> +++ polly/trunk/lib/Exchange/JSONExporter.cpp Sat May 23 00:14:09 2015
> @@ -102,9 +102,7 @@ Json::Value JSONExporter::getJSON(Scop &
>      root["location"] = Location;
>    root["statements"];
>
> -  for (Scop::iterator SI = S.begin(), SE = S.end(); SI != SE; ++SI) {
> -    ScopStmt *Stmt = *SI;
> -
> +  for (auto &Stmt : S) {
>      Json::Value statement;
>
>      statement["name"] = Stmt->getBaseName();
> @@ -234,10 +232,10 @@ bool JSONImporter::runOnScop(Scop &S) {
>
>    int index = 0;
>
> -  for (Scop::iterator SI = S.begin(), SE = S.end(); SI != SE; ++SI) {
> +  for (auto &Stmt : S) {
>      Json::Value schedule = jscop["statements"][index]["schedule"];
>      isl_map *m = isl_map_read_from_str(S.getIslCtx(),
> schedule.asCString());
> -    isl_space *Space = (*SI)->getDomainSpace();
> +    isl_space *Space = Stmt->getDomainSpace();
>
>      // Copy the old tuple id. This is necessary to retain the user
> pointer,
>      // that stores the reference to the ScopStmt this schedule belongs to.
> @@ -248,7 +246,7 @@ bool JSONImporter::runOnScop(Scop &S) {
>        m = isl_map_set_dim_id(m, isl_dim_param, i, id);
>      }
>      isl_space_free(Space);
> -    NewSchedule[*SI] = m;
> +    NewSchedule[&*Stmt] = m;
>      index++;
>    }
>
> @@ -262,17 +260,13 @@ bool JSONImporter::runOnScop(Scop &S) {
>      return false;
>    }
>
> -  for (Scop::iterator SI = S.begin(), SE = S.end(); SI != SE; ++SI) {
> -    ScopStmt *Stmt = *SI;
> -
> -    if (NewSchedule.find(Stmt) != NewSchedule.end())
> -      Stmt->setSchedule(NewSchedule[Stmt]);
> +  for (auto &Stmt : S) {
> +    if (NewSchedule.find(&*Stmt) != NewSchedule.end())
> +      Stmt->setSchedule(NewSchedule[&*Stmt]);
>    }
>
>    int statementIdx = 0;
> -  for (Scop::iterator SI = S.begin(), SE = S.end(); SI != SE; ++SI) {
> -    ScopStmt *Stmt = *SI;
> -
> +  for (auto &Stmt : S) {
>      int memoryAccessIdx = 0;
>      for (MemoryAccess *MA : *Stmt) {
>        Json::Value accesses = jscop["statements"][statementIdx]["accesses"]
>
> Modified: polly/trunk/lib/Transform/ScheduleOptimizer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Transform/ScheduleOptimizer.cpp?rev=238090&r1=238089&r2=238090&view=diff
>
> ==============================================================================
> --- polly/trunk/lib/Transform/ScheduleOptimizer.cpp (original)
> +++ polly/trunk/lib/Transform/ScheduleOptimizer.cpp Sat May 23 00:14:09
> 2015
> @@ -467,7 +467,7 @@ bool IslScheduleOptimizer::runOnScop(Sco
>
>    S.markAsOptimized();
>
> -  for (ScopStmt *Stmt : S) {
> +  for (auto &Stmt : S) {
>      isl_map *StmtSchedule;
>      isl_set *Domain = Stmt->getDomain();
>      isl_union_map *StmtBand;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/b2d92e06/attachment.html>


More information about the llvm-commits mailing list