[PATCH] D53865: [LoopVectorizer] Improve computation of scalarization overhead.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.



In D53865#1336940 <https://reviews.llvm.org/D53865#1336940>, @hsaito wrote:

> > Unfortunately, this could not be done with computeInstDiscount() as a simple extension of (1), since memory accesses are dealt with before anything else.
>
> If we change "memory widening decision" from "hard decision" to "soft decision to be finalized after collect scalars", we might be able to solve that. In general, I don't think this is any different from data flow based optimization problem
>
> - and the standard solution there is do this iteratively. So, some form of going back and revise the prior decision is inevitable if we want to improve this.


I was not sure this is a natural extension of the current algorithm. Currently, the memory widening decisions are first made and then the loop uniforms and scalars are collected based on this. Are you saying that changing the decision to 'scalarize' from e.g. 'widen' could be done without affecting the set of uniforms/scalars? Perhaps it could be... Scalarizing a widened access might just mean some extra accesses with immediate offsets, which would not require any changes to the induction variables...

> In any case, the newer code you have written is more generically applicable, and the improvements needed on top of that is also generically applicable. Best of all, we don't have to teach downstream optimizers about the weirdness in cost modeling. 
>  That's what I was aiming for in reviewing your change.

Yes, I agree it looks better now :-)

> Is the uploaded code ready for review? Or shall we continue discussing the approach?

It's quite ready, except for our discussion above.

Also, in the spirit of keeping LoopVectorizer as clean as possible, I think we should have some more people agreeing to this as a general improvement. If this would be the case, I would be happy to commit, but I can't say at the moment if this is really improving things enough to go in... It may be that this is mostly a theoretical improvement which may or may not be worth having, depending on the complexity of the patch...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53865/new/

https://reviews.llvm.org/D53865