[PATCH] Improve ScalarEvolutionExpander to try harder not to create unnecessary induction variables in dominating loops
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.
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.
// check whether we can reuse this phi
// check expandedaddrecphi
// make sure we can hoist the increment above the insert pos
// check isnormaladdrec
//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...
Size: 11921 bytes
Desc: not available
More information about the llvm-commits