This is an archive of the discontinued LLVM Phabricator instance.

[LV] Fix to pr37515, FP primary induction crashes LV
Needs ReviewPublic

Authored by hsaito on May 22 2018, 12:32 PM.

Details

Summary

LV never had an ability to convert FP primary induction to INT primary induction, but it has been lacking an appropriate safety check.
Whether it's legal to convert FP primary induction to INT primary induction and if so under what conditions are debatable, but bailing
out when it's not proven safe (and currently never proven to be safe as far as LV's existing code is concerned) is a valid thing to do.

The bug may be introduced (or exposed depending on how you see it) by FP induction support ( approx. 2yrs ago?).

Diff Detail

Event Timeline

hsaito created this revision.May 22 2018, 12:32 PM
hsaito added inline comments.May 22 2018, 12:36 PM
lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
729

In pr37515, Ayal suggested going just "if (!WidestIndTy)". I agree.

He also suggested adding an assert for non-NULL IdxTy in the following place. Also makes sense.

This line in InnerLoopVectorizer::getOrCreateTripCount():

Type *IdxTy = Legal->getWidestInductionType();

Will add a LIT test.

hsaito updated this revision to Diff 148090.May 22 2018, 1:34 PM

LIT test added. ORE messaging doesn't seem to be working here (I see just "loop not vectorized" without the rest of the string), but that should be fixed separately.