[PATCH] Improve ScalarEvolutionExpander to try harder not to create unnecessary induction variables in dominating loops

Arnold Schwaighofer aschwaighofer at apple.com
Fri Feb 14 15:41:06 PST 2014

On Feb 13, 2014, at 2:31 PM, Andrew Trick <atrick at apple.com> wrote:
> This has always been a problem with SCEVExpander, hence LSR. Your fix looks like a major improvement. Unfortunately, making SCEVExpander do reasonable things requires a lot of code complexity.
> So, why is this gated by LSRMode? I think it should be on all the time.
> Why is this separate from the existing search for reusable phis just above it? I think this is just another set of conditions similar to what we're already searching for. Can we combine the search but split logic into helpers? I think there are overlapping concerns between the existing logic and yours. It would be nice to see these issue cleanly separated:
> - In LSRMode, if this is the current LSR loop, make sure it looks like an LSR-ish phi
> - Make sure the expanded expression dominates it users (check isNormalAddRecExprPHI only when necessary)
> - Find out of the phi's AddRec can be easily/cheaply converted into the one we're requesting.
> -Andy

Thanks for the review.

Changed so that we have one loop:

for ‘phi' in ‘loop' of expanded addrec ’normalize':
  ismatchingscev = scev(phi) == scev(normalize)
  // bail out early for non-exact-matching scevs if we don’t know the loop we are expanding dominates the loop we are expanding into.
  if (!ismatchingscev)
    if(!IVIncInsertLoop ||
       properlyDominates(loop, IVIncInsertLoop)

  // check whether we can reuse this phi
    // check expandedaddrecphi
    // make sure we can hoist the increment above the insert pos
    // check isnormaladdrec

  if (ismatchingscev)
    //sucess break out of loop.
    // check whether we can use the phi by truncating and/or inverting.

if (found match)
  // hoist increment above insertpos if required
  return the phi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SCEVExpander-Try-hard-not-to-create-derived-inductio.patch
Type: application/octet-stream
Size: 11921 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140214/39b76fc8/attachment.obj>

More information about the llvm-commits mailing list