[PATCH] D15068: ScopInfo: Replace while/iterator construct with std::remove_if

[David Blaikie via llvm-commits]

On Sun, Nov 29, 2015 at 9:36 PM, Tobias Grosser via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> grosser created this revision.
> grosser added reviewers: jdoerfert, Meinersbur.
> grosser added subscribers: llvm-commits, pollydev.
>
> The use of C++'s high-level iterator functionality instead of two while
> loops
> and explicit iterator handling improves readability of this code.
>
> This passes LNT -polly-process-unprofitable for me.
>
> Proposed-by: Michael Kruse <llvm at meinersbur.de>
>
> http://reviews.llvm.org/D15068
>
> Files:
>   lib/Analysis/ScopInfo.cpp
>
> Index: lib/Analysis/ScopInfo.cpp
> ===================================================================
> --- lib/Analysis/ScopInfo.cpp
> +++ lib/Analysis/ScopInfo.cpp
> @@ -1464,28 +1464,18 @@
>  void ScopStmt::dump() const { print(dbgs()); }
>
>  void ScopStmt::removeMemoryAccesses(MemoryAccessList &InvMAs) {
> -
> -  // Remove all memory accesses in @p InvMAs from this statement together
> -  // with all scalar accesses that were caused by them. The tricky
> iteration
> -  // order uses is needed because the MemAccs is a vector and the order in
> -  // which the accesses of each memory access list (MAL) are stored in
> this
> -  // vector is reversed.
> +  // Remove all memory accesses in @p InvMAs from this statement
> +  // together with all scalar accesses that were caused by them.
>    for (MemoryAccess *MA : InvMAs) {
> -    auto &MAL = *lookupAccessesFor(MA->getAccessInstruction());
> -    MAL.reverse();
> -
> -    auto MALIt = MAL.begin();
> -    auto MALEnd = MAL.end();
> -    auto MemAccsIt = MemAccs.begin();
> -    while (MALIt != MALEnd) {
> -      while (*MemAccsIt != *MALIt)
> -        MemAccsIt++;
> -
> -      MALIt++;
> -      MemAccs.erase(MemAccsIt);
> -    }
> -
> +    auto Predicate = [MA](MemoryAccess *Acc) -> bool {
>

Just capture everything by ref here ^ ("[&]") since the lambda is being
used in the scope (it's not leaking out via std::function, etc) so
everything will be valid & it should just be treated like a normal scope.




> +      return (Acc == MA) ||
>

Drop unneeded parens ^


> +             (Acc->isWrite() &&
> +              Acc->getAccessInstruction() == MA->getAccessInstruction());
> +    };
> +    MemAccs.erase(std::remove_if(MemAccs.begin(), MemAccs.end(),
> Predicate),
>

Probably just define the lambda directly inline without giving it a name?


> +                  MemAccs.end());
>      InstructionToAccess.erase(MA->getAccessInstruction());
> +    auto &MAL = *lookupAccessesFor(MA->getAccessInstruction());
>      delete &MAL;
>

Drop the local variable & pass the result of lookupAccessesFor to delete,
without the dereference and address-of usage?


>    }
>  }
>
>
>
> _______________________________________________
> 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/20151129/7f8d9111/attachment.html>