addressed clang-format issue

Addressed tidy warnings

rebased

Will get back to pushing this patch in a few weeks (unfortunately didn't make it before going away on vacation).

updated a comment, as pointed out in the review

Thanks, Ayal! Incorporated all your comments.

Addressed review comments.

(accidentally uploaded without context)

updated formatting

In D104750#2834571, @lebedev.ri wrote:

Costmodel is wrong: if there are gaps, you need to load the vector of original values, and insert the non-gap elements into it.

Thanks for taking a look! Please see responses below.

In D57180#1376068, @fhahn wrote:

I plan on having a look later this week. I am a little worried that the checks in-line here are already quite complex and I would like to have a think if that could be improved in some way.

Comments addressed. Thanks!

Addressed comments.
Also added a test with stride 3.

Just minor comments on the tests.
LGTM.

Addressed comments. See couple responses below. Thanks!

updated to top of trunk.

Comment addressed, thanks.

Addressed Ayal's comments. Thanks!

Thanks!

Hopefully I can delegate the review to Diego...
Thanks for the fix, Andrei

LGTM. Thanks for the fix.

Thanks so much for all your help with this work!

Addressed Ayal's and Silviu's comments.

Dropped the parts that are uploaded for review separately (D40641, D40883), and hopefully addressed Silviu's last comments.

ping

Thanks Florian. Uploaded the formatting fix.

Hi Ayal,

(uploaded a fix to LoopUtils:getCastsForInductionPHI())

Hi Silviu,

Hi Silviu,

Hi Silviu,
I started to try out the approach you suggested, and I realized that our assumption doesn't hold... (see response to inlined comment).
Thanks,
Dorit

Addressed Ayal's comments.
Have yet to address Silviu's comments.

Yes, IndVarSimpify wouldn't fix this issue, but I was thinking more of using the techniques there that use the SCEV expressions to find these cases instead of doing the pattern matching (see the inline comment).

Hi Silviu,

Thanks Ayal. Incorporated your suggestions.

ping^2

You missed just one mcpu=skylake :)
LGTM with this change

I think it would be nice to make the testcases smaller; Right now you have something like this:
for (…) {
Dst[2*i] = Dst[2*i] + Src[2*i] * k
Dst[2*i+1] = Dst[2*i+1] + Src[2*i+1] * k
}
...which actually tests both strided loads and strided stores.
So you could either use one test to check both store and load costs (and even then you probably don't need both a mul and an add just to check memops costs).
Or if you want to separate the load and store cases, the Load test could be something like:
for (…) {
s += Src[2*i]
s += Src[2*i+1]
}
The Store test could be something like:
For(…){

Dst[2*i] = k1;
Dst[2*i+1] = k2;

}

ping :)

Incorporated Ayal's comments. Thanks!

Hi Silviu,

LGTM with the last couple of comments.

AVX512 side of things now also looks good to me (with the tiny comments below).

The AVX2 changes look ok to me now.
A couple comments about the AVX512 changes below.

Hi Silviu,

My only main concerns are with respect to interleave-group with gaps (see below), and the fact that we don't distinguish the AVX2 from the AVX512 case (also see below). Just minor comments beyond that.

Looks fine to me (with a couple very minor dbg reports improvements).
@Ayal/@hfinkel - what do you say?