[PATCH] D33121: [SCEV] Fix getAddExpr for recurrencies from different loops

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
Herald added a subscriber: mzolotukhin.

The way getAddExpr worked with simplification of AddRecExpr's was not entirely correct.
The old algorithm just tried to find the first recurrence among operands and tried to fold
it with other operands with respect to this loop, what is incorrect. If we have more than
one AddRecExpr operand, there is no guarantee that the recurrence can be picked in
arbitrary way. Consider case:

  for (...) phi1 = ...
  for (...) {
    phi2 = ...
    sum = phi1 + v + phi2
  }

If we follow the existing algorithm, we pick the recurrence for phi1 and try to fold other
values with it, so that in result we have some recurrency {X,+,Y}<loop1>. But in fact, this
is wrong, at least because phi2 is not visible from the loop1. In fact, sum varies within the
loop 2. If we have more than one phi that are being added, we should do our transformatio
 with respect to the loop from which all other values are visible, i.e. the 'bottom-most' loop
in terms of domination.

This patch fixes this logic to pick the right recurrence as the base for folding when we try to
simplify AddExpr for them. It also adds some regression tests that demonstrate that the old
implementation of this algorithm does a wrong thing and that it can lead to crash. All previous
functionality preserves: the scope of getAddExpr is not reduced by this change.


https://reviews.llvm.org/D33121

Files:
  lib/Analysis/ScalarEvolution.cpp
  test/Analysis/ScalarEvolution/different-loops-recs.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33121.98743.patch
Type: text/x-patch
Size: 21954 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170512/a50762c2/attachment.bin>