This is an archive of the discontinued LLVM Phabricator instance.

[LV] Identify more induction PHIs by coercing expressions to AddRecExprs
ClosedPublic

Authored by sbaranga on Feb 11 2016, 10:12 AM.

Details

Summary

Some PHIs can have expressions that are not AddRecExprs due to the presence
of sext/zext instructions. In order to prevent the Loop Vectorizer from
bailing out when encountering these PHIs, we now coerce the SCEV
expressions to AddRecExprs using SCEV predicates (when possible).

We only do this when the alternative would be to not vectorize.

Diff Detail

Event Timeline

sbaranga updated this revision to Diff 47678.Feb 11 2016, 10:12 AM
sbaranga retitled this revision from to [LV] Identify more induction PHIs by coercing expressions to AddRecExprs.
sbaranga updated this object.
sbaranga added reviewers: anemet, mzolotukhin.
sbaranga added a subscriber: llvm-commits.
sbaranga updated this revision to Diff 53420.Apr 12 2016, 9:39 AM

Perform a rebase of the patch.

Only add SCEV predicates after we know that we couldn't do anything else.

However, we still need to identify induction variables before trying to find
first-order recurrences. Therefore, we will now classify in Phis in the following
order:

  1. induction
  2. first-order recurrence
  3. induction, with the help of SCEV predicates.
  4. give up
anemet requested changes to this revision.Apr 19 2016, 4:28 PM
anemet edited edge metadata.
anemet added inline comments.
lib/Transforms/Vectorize/LoopVectorize.cpp
4594–4627

Why did you choose a lambda rather than a helper for this? This is a pretty big function and the lambda is not small either. Unless there is a strong reason I would prefer factoring this part out into a helper.

test/Transforms/LoopVectorize/induction.ll
171

Please comment which induction PHI you mean here? Same on the other testcase.

This revision now requires changes to proceed.Apr 19 2016, 4:28 PM
sbaranga added inline comments.Apr 20 2016, 4:02 AM
lib/Transforms/Vectorize/LoopVectorize.cpp
4594–4627

There's no strong reason for this (it just avoided passing a lot of arguments). I'll convert it to a helper function.

test/Transforms/LoopVectorize/induction.ll
171

Sure! (it's %sphi in both tests)

sbaranga updated this revision to Diff 54372.Apr 20 2016, 8:00 AM
sbaranga edited edge metadata.

Convert the lambda into a helper function.

Ping?

-Silviu

anemet accepted this revision.May 4 2016, 9:38 AM
anemet edited edge metadata.

Sorry about the delay, this LGTM.

include/llvm/Transforms/Utils/LoopUtils.h
296–297

Please add function comment. Would be nice to also add comment for the original function. Thanks!

lib/Transforms/Vectorize/LoopVectorize.cpp
1401

"main main"

4533–4537

Please peel off the NFC part of splitting out the helper into a separate post-commit review change.

This revision is now accepted and ready to land.May 4 2016, 9:38 AM
sbaranga updated this revision to Diff 56294.May 5 2016, 8:25 AM
sbaranga edited edge metadata.

Update the code after the helper creation part was committed in r268632.

sbaranga closed this revision.May 5 2016, 8:26 AM

Thanks, committed in r268633.

-Silviu