Seems reasonable to me. LGTM

Thanks, LGTM!

LGTM! (with one nit)

Thanks, LGTM!

The initial intent was to make sure that we are forcing the operands to not be VFP registers, so I would have expected to get a vadd.f32 r0, r0, r0. However, since the instruction is invalid it doesn't make a good test (as you observed).

IIRC the contents of the asm string shouldn't matter here, and we would expect to get an error in the assembler instead when trying to assemble the output (which is fine, since this is a misuse of the X constraint).

For pointer-with-unknown-bounds.ll, I think the point was just checking that we don't vectorize in the case where the SCEV expressions for the pointers are not affine.

Hi Anna,

LGTM

In D49461#1166719, @hsaito wrote:

In D49461#1166158, @sbaranga wrote:

Seems reasonable to me.

Ideally we should be handling the phi node case (which is why the unreachable was there), but crashing doesn't seem like the right thing to do.

Personally, I see a value in differentiating explicitly known handling versus just being conservative ----- but I don't mind converting that into a comment instead if that's preferred.

Seems reasonable to me.

If this is no longer only used by Cortex-A15, I think the regression tests need to be updated to use the new feature instead of the passing the cpu.

LGTM, thanks!

LGTM with one minor nit.

Thanks for submitting the fix! Please see the inline comments.

LGTM!

Hi Dorit,

In D38948#929665, @dorit wrote:

IIUC, what IndVarSimply does is call the rewriter and then fix the users of the induction; This is similar in effect to what we do: We don't need to call the PSCEV rewriter again, we already have the nice AddRec for the induction phi (isInductionPhi() had already obtained it); The vectorization of the induction phi proceeds unchanged; The only thing we are adding is the def-use wiring so that in the vectorized loop, any users of the cast instructions will be users of the vectorized phi. We never vectorize the casts, and we never actively remove them -- they will end up dead code in the vectorized loop because they will not be used.

Sorry for not having a look earlier.

Hi Anna,

Thanks! Committed in r313012.

Address comments received so far.

Sorry for not replying earlier. It looks like there are only minor changes left? I plan to push an update after this gets unblocked.

It looks ok to me, thanks!

This generally makes sense to me!

Sorry for the delay, I missed the last update. I have a few minor suggestions, but otherwise I think it generally looks good.
I think Sanjoy still needs to approve this before it can go in.

We already have the IR in misched-fusion-aes.ll that can be used for microbenchmarks (just call the functions in a loop)? I think that should be theoretically enough for an evaluation.

Hi Florian,

nitpick: you should run a spell checker over the patch.

Hi Dorit,

LGTM

Renaming Force to Assume.

Hi Dorit,

This looks straight-forward to me. LGTM.

LGTM!

Thanks, both! r288253

Update variable names names in the test and make it more readable overall.

It looks like gcc should have been emitting an error here instead of the "sorry, not implemented".

Thanks, that makes sense. I also wasn't aware of no-implicit-float.

Update regression tests.

Thanks for making the changes! LGTM

Yes, now that I think about it skipping case c) makes sense.

LGTM for the SCEV part.

In D25276#574171, @mssimpso wrote:

Intuitively it seems like we would want to use getPtrStride in the interleaved access analysis the same way we do in isConsecutivePtr. Is the issue here that we might add additional run-time checks that we aren't currently? We already call isConsecutivePtr for the pointer operand of every load/store in the loop. Would the additional checks then be duplicates?

In D25276#573001, @dorit wrote:

I see. Thanks.

So in this case, don't we have a problem of potentially adding runtime SCEV assumptions without ever checking if we exceed the threshold? getPtrStride is called with Assume=true several times from the cost-model stage and from the actual vectorization transformation stage, via the call to isConsecutivePtr. Looks like these call sites are past the point when we check whether we exceeded the threshold for runtime SCEV assumptions...?

LGTM

In D25276#567145, @dorit wrote:

Agreed?

I just had a brief look through this, but I don't think LAA changes are correct (see the inline comment).
Also if there are non-functional changes here, could you split them into another review (this would make things easier to understand)?

In D25276#565553, @dorit wrote:

In D25276#565321, @sbaranga wrote:

Thanks Silviu for the detailed explanation.

ShouldCheckWrap indicates if we need to check if the pointer is wrapping in order to identify this as a strided pointer. This is mainly needed for dependence analysis.

So I take it we are fine with ShouldCheckWrap=false here (being past the dependence testing stage).

ShouldCheckWrap indicates if we need to check if the pointer is wrapping in order to identify this as a strided pointer. This is mainly needed for dependence analysis.

Very nice! LGTM

I've investigated using MIR for getting a test case. The problem that I currently have with this is that I'm unable to run the PEI pass (where the problem is happening). If I try to do this I get a segfault in the stack protector pass. The problem seems to be that the TargetMachine is null. I'm not sure if this is something specific to PEI - but there doesn't seem to be any other regression tests that tries to run just this pass.

In D24130#534839, @qcolombet wrote:

Have you investigate using the mir representation to produce a test case?

Thanks, Hal! r280698

Thanks, James! Committed in r278002.