[PATCH] D22778: Add Loop Sink pass to reverse the LICM based of basic block frequency.

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 13:17:03 PDT 2016


>
>
>
> ================
> Comment at: lib/Transforms/Scalar/LoopSink.cpp:133
> @@ +132,3 @@
> +      INS.push_back(&I);
> +  }
> +  for (auto I : INS) {
> ----------------
> We need reverse iterator because instructions in the back of the BB may
> depend on the instructions in the front, thus it needs to be sunk first
> before other instructions can be sunk.
>

I understand this, i'm asking "why are you using a smallvector and copying
them instead of just using reverse iterator directly"?

> ================
> Comment at: lib/Transforms/Scalar/LoopSink.cpp:199
> @@ +198,3 @@
> +
> +    int i = 0;
> +    for (BasicBlock *N : SinkBBs) {
> ----------------
> SinkBBs.size() == 0 check is already moved above the total frequency
> check, so it will not reach here.
>
> The i==0 check is to distinguish between the first SinkBB (that we use
> move instead of insert) and the later SinkBB (that we make a copy for each
> insert).


Right, i'm just suggesting you move the first SinkBB logic out of the loop
becuase it makes the loop a lot easier to understand.



>
>
> https://reviews.llvm.org/D22778
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160819/571604d0/attachment.html>


More information about the llvm-commits mailing list