[polly] r308152 - ScopInfo: Remove not-in-DomainMap statements in separate function

[Michael Kruse via llvm-commits]

A post-commit review.


2017-07-17 1:55 GMT+02:00 Tobias Grosser via llvm-commits
<llvm-commits at lists.llvm.org>:
> Author: grosser
> Date: Sun Jul 16 16:55:38 2017
> New Revision: 308152
>
> URL: http://llvm.org/viewvc/llvm-project?rev=308152&view=rev
> Log:
> ScopInfo: Remove not-in-DomainMap statements in separate function
>
> This separates ScopBuilder internal and ScopBuilder external functionality.

I don't quite understand the motivation. The refactoring of
removeStmts to be usable in more contexts makes sense (although I
think just providing a `removeStmt()` method that could be called
individually would have been a simpler solution than a callback)

If the purpose was to separate removeStmtNotInDomainMap() which is
only used by ScopBuilder, why is it still a method of ScopInfo instead
one of ScopBuilder?


> Modified:
>     polly/trunk/include/polly/ScopInfo.h
>     polly/trunk/lib/Analysis/ScopBuilder.cpp
>     polly/trunk/lib/Analysis/ScopInfo.cpp
>
> Modified: polly/trunk/include/polly/ScopInfo.h
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopInfo.h?rev=308152&r1=308151&r2=308152&view=diff
> ==============================================================================
> --- polly/trunk/include/polly/ScopInfo.h (original)
> +++ polly/trunk/include/polly/ScopInfo.h Sun Jul 16 16:55:38 2017
> @@ -2148,6 +2148,19 @@ private:
>    /// all memory accesses have been modeled and canonicalized.
>    void assumeNoOutOfBounds();
>
> +  /// Remove statements from the list of scop statements.
> +  ///
> +  /// @param ShouldDelete A function that returns true if the statement passed
> +  ///                     to it should be deleted.
> +  void removeStmts(std::function<bool(ScopStmt &)> ShouldDelete);
> +
> +  /// Removes @p Stmt from the StmtMap.
> +  void removeFromStmtMap(ScopStmt &Stmt);
> +
> +  /// Removes all statements where the entry block of the statement does not
> +  /// have a corresponding domain in the domain map.
> +  void removeStmtNotInDomainMap();
> +
>    /// Mark arrays that have memory accesses with FortranArrayDescriptor.
>    void markFortranArrays();
>
>
> Modified: polly/trunk/lib/Analysis/ScopBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopBuilder.cpp?rev=308152&r1=308151&r2=308152&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopBuilder.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopBuilder.cpp Sun Jul 16 16:55:38 2017
> @@ -982,6 +982,7 @@ void ScopBuilder::buildScop(Region &R, A
>
>    // Remove empty statements.
>    // Exit early in case there are no executable statements left in this scop.
> +  scop->removeStmtNotInDomainMap();
>    scop->simplifySCoP(false);
>    if (scop->isEmpty())
>      return;
>
> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=308152&r1=308151&r2=308152&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Sun Jul 16 16:55:38 2017
> @@ -3728,13 +3728,37 @@ void Scop::assumeNoOutOfBounds() {
>        Access->assumeNoOutOfBound();
>  }
>
> -void Scop::simplifySCoP(bool AfterHoisting) {
> +void Scop::removeFromStmtMap(ScopStmt &Stmt) {
> +  if (Stmt.isRegionStmt())
> +    for (BasicBlock *BB : Stmt.getRegion()->blocks())
> +      StmtMap.erase(BB);
> +  else
> +    StmtMap.erase(Stmt.getBasicBlock());
> +}
> +
> +void Scop::removeStmts(std::function<bool(ScopStmt &)> ShouldDelete) {
>    for (auto StmtIt = Stmts.begin(), StmtEnd = Stmts.end(); StmtIt != StmtEnd;) {
> -    ScopStmt &Stmt = *StmtIt;
> +    if (!ShouldDelete(*StmtIt)) {
> +      StmtIt++;
> +      continue;
> +    }
>
> +    removeFromStmtMap(*StmtIt);
> +    StmtIt = Stmts.erase(StmtIt);
> +  }
> +}
> +
> +void Scop::removeStmtNotInDomainMap() {
> +  auto ShouldDelete = [this](ScopStmt &Stmt) -> bool {
> +    return !this->DomainMap[Stmt.getEntryBlock()];

DomainMap.count(Stmt.getEntryBlock())
would not create an intry if there is none yet.

> +  };
> +  removeStmts(ShouldDelete);
> +}
> +
> +void Scop::simplifySCoP(bool AfterHoisting) {
> +
> +  auto ShouldDelete = [AfterHoisting](ScopStmt &Stmt) -> bool {
>      bool RemoveStmt = Stmt.isEmpty();
> -    if (!RemoveStmt)
> -      RemoveStmt = !DomainMap[Stmt.getEntryBlock()];
>
>      // Remove read only statements only after invariant load hoisting.
>      if (!RemoveStmt && AfterHoisting) {
> @@ -3749,21 +3773,10 @@ void Scop::simplifySCoP(bool AfterHoisti
>
>        RemoveStmt = OnlyRead;
>      }
> +    return RemoveStmt;
> +  };
>
> -    if (!RemoveStmt) {
> -      StmtIt++;
> -      continue;
> -    }
> -
> -    // Remove the statement because it is unnecessary.
> -    if (Stmt.isRegionStmt())
> -      for (BasicBlock *BB : Stmt.getRegion()->blocks())
> -        StmtMap.erase(BB);
> -    else
> -      StmtMap.erase(Stmt.getBasicBlock());
> -
> -    StmtIt = Stmts.erase(StmtIt);
> -  }
> +  removeStmts(ShouldDelete);
>  }
>
>  InvariantEquivClassTy *Scop::lookupInvariantEquivClass(Value *Val) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits