[IndVars] Fix PR24783.
authorSanjoy Das <sanjoy@playingwithpointers.com>
Tue, 15 Sep 2015 23:45:39 +0000 (23:45 +0000)
committerSanjoy Das <sanjoy@playingwithpointers.com>
Tue, 15 Sep 2015 23:45:39 +0000 (23:45 +0000)
commite1269fa6d77239b191f2bc1ad79bf9002e7f4367
tree974da4197c345735a19c7a8f804a22bd265fba5c
parenta67a10589497dacad88043be4ee5f6753e380d41
[IndVars] Fix PR24783.

In `IndVarSimplify::ExpandSCEVIfNeeded`,
`SCEVExpander::findExistingExpansion` may return an `llvm::Value` that
differs in type from the SCEV it was asked to find an expansion for (but
computes the same value).  In such cases, we fall back on
`expandCodeFor`; and rely on LLVM to CSE the two equivalent
expressions (different only by a no-op cast) into a single computation.

I tried a few other approaches to fixing PR24783, all of which turned
out to be more complex than this current version:

 1. Move the `ExpandSCEVIfNeeded` logic into `expandCodeFor`.  This got
    problematic because currently we do not pass in the `Loop *` into
    `expandCodeFor`.  Changing the interface to do this is a more
    invasive change, and really does not make much semantic sense unless
    the SCEV being passed in is an add recurrence.

    There is also the problem of `expandCodeFor` being used in places
    other than `indvars` -- there may be performance / correctness
    issues elsewhere if `expandCodeFor` is moved from always generating
    IR from scratch to cache-like model.

 2. Have `findExistingExpansion` only return expression with the correct
    type.  This would make `isHighCostExpansionHelper` and thus
    `isHighCostExpansion` more conservative than necessary.

 3. Insert casts on the value returned by `findExistingExpansion` if
    needed using `InsertNoopCastOfTo`.  This is complicated because
    `InsertNoopCastOfTo` depends on internal state of its
    `SCEVExpander` (specifically `Builder.GetInserPoint()`), and this
    may not be set up when `ExpandSCEVIfNeeded` is called.

 4. Manually insert casts on the value returned by
    `findExistingExpansion` if needed using `InsertNoopCastOfTo` via
    `CastInst::Create`.  This is probably workable, but figuring out the
    location where the cast instruction needs to be inserted has enough
    edge cases (arguments, constants, invokes, LCSSA must be preserved)
    makes me feel what I have right now is simplest solution.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@247749 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Transforms/Scalar/IndVarSimplify.cpp
test/Transforms/IndVarSimplify/pr24783.ll [new file with mode: 0644]