[PATCH] D51286: [IndVars] Fix usage of SCEVExpander in IndVarSimplify to not mess with SCEVConstant. PR38674

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: etherzhhb, greened, sanjoy, uabelho, reames.
Herald added a subscriber: javed.absar.

This patch removes the function `expandSCEVIfNeeded` which behaves not as
it was intended. This function tries to make a lookup for exact existing expansion
and only goes to normal expansion via `expandCodeFor` if this lookup hasn't found
anything. As a result of this, if some instruction above the loop has a `SCEVConstant`
SCEV, this logic will return this instruction when asked for this `SCEVConstant` rather
than return a constant value. This is both non-profitable and in some cases leads to
breach of LCSSA form (as in PR38674).

Whether or not it is possible to break LCSSA with this algorith and with some
non-constant SCEVs is still in question, this is still being investigated. I wasn't
able to construct such a test so far, so maybe this situation is impossible. If it is,
it will go as a separate fix.

Rather than do it, it is always correct to just invoke `expandCodeFor` unconditionally:
it will check corner cases like constants in the first place and then will make the same
lookup among existing values in the map. So it should not be worse in any case.


https://reviews.llvm.org/D51286

Files:
  lib/Transforms/Scalar/IndVarSimplify.cpp
  test/Transforms/IndVarSimplify/pr38674.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51286.162624.patch
Type: text/x-patch
Size: 7882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180827/d4856b71/attachment.bin>