[PATCH] D84951: [LV] Try to sink users recursively for first-order recurrences.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 14 13:31:53 PDT 2021


fhahn updated this revision to Diff 345544.
fhahn added a comment.

Rebased, update to also avoid sinking instructions that may read from memory, as we cannot move them over ones with side-effects, move sort to isFirstOrderRecurrence.

In D84951#2753861 <https://reviews.llvm.org/D84951#2753861>, @Ayal wrote:

> How about making SinkAfter a MapVector instead of DenseMap (to have its iteration order match insertion order) and insert interdependent sink candidates in the desired order?
> How about traversing all insns from phi to previous, in order, checking if any is using the phi or a prior insn that was sunk? E.g., by maintaining the set of their users.
> This might be clearer, and avoid the sorting.

I'm not sure if it is possible to maintain the order by dominance in the map as we go along, because we an instruction could be used by multiple others and we may visit them in reverse dominance order (relative to each other), so we would miss the fact we need to 'fix' the order of the first instruction. I think it is also difficult & expensive to change the order of multiple entries in the map vector.

I added a few more interesting scenarios as tests in 68d52f0dbe2e <https://reviews.llvm.org/rG68d52f0dbe2ef1fd36850c2a637d6db70f509b32> and c62f984814c4 <https://reviews.llvm.org/rGc62f984814c4994911dee75bad28cb7346ca3c07>.

I *think* however it should be enough to sort only sort the instructions to sink per FOR phi, as we currently allow sinking for each instruction only once. That should certainly be better, as we have less to sort overall and everything is contained in IVDescriptor.cpp. The new code already maintains a vector of instructions to sink, so the sort fits in quite nicely. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84951/new/

https://reviews.llvm.org/D84951

Files:
  llvm/include/llvm/Analysis/IVDescriptors.h
  llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h
  llvm/lib/Analysis/IVDescriptors.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Transforms/LoopVectorize/first-order-recurrence-complex.ll
  llvm/test/Transforms/LoopVectorize/first-order-recurrence.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84951.345544.patch
Type: text/x-patch
Size: 30510 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210514/24591391/attachment.bin>


More information about the llvm-commits mailing list