[PATCH] D23704: [GraphTraits] Make nodes_iterator dereference to NodeType*

Tim Shen via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 19:17:45 PDT 2016


On Fri, Aug 19, 2016, 14:47 Daniel Berlin <dberlin at dberlin.org> wrote:

> dberlin added a subscriber: dberlin.
>
> ================
> Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:212
> @@ -211,3 +211,3 @@
>    // ensures that all bases of a candidate are in Candidates when we
> process it.
>    for (auto Node = GraphTraits<DominatorTree *>::nodes_begin(DT);
>         Node != GraphTraits<DominatorTree *>::nodes_end(DT); ++Node) {
> ----------------
> This is actually not preorder :)
> It's depth first.
>
> and could be replaced with the depth first iterator
>
>
> ================
> Comment at: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:1155
> @@ -1154,3 +1154,3 @@
>         Node != GraphTraits<DominatorTree *>::nodes_end(DT); ++Node) {
> -    BasicBlock *BB = Node->getBlock();
> +    BasicBlock *BB = (*Node)->getBlock();
>      for (auto I = BB->begin(); I != BB->end(); ) {
> ----------------
> Ditto, this is just depth_first, and should use that.
>
>
> ================
> Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:680
> @@ -680,2 +679,3 @@
> +    for (auto &I : *(*node)->getBlock())
>        allocateCandidatesAndFindBasis(&I);
>    }
> ----------------
> This whole glop is just a fancy way of saying:
>
>  for (const auto *Node : depth_first(DT)) {
> allocateCandidatesAndFindBasis(I->getBlock()); }
>
> I'll fix it if you don't :)
>

Sorry, I missed your email earlier :P

Thanks for the fix (and partial revert)!


>
>
> https://reviews.llvm.org/D23704
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160820/3e6fac8f/attachment.html>


More information about the llvm-commits mailing list