[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for pointing this all out!

I am not entirely sure yet what to think about all this as I am new to the loop pragma business, but I think it looks inconsistent to me!

I think I find `allowReordering()` a little bit ugly, because it is also checking `getWidth() > 1`, and I am probably expecting the allowReordering decision to be based on 1 thing (the Force). But in the current implementation that probably makes sense though if `vectorizer_width` doesn't set `vectorizer.enable` (again, which is not what I would expect). Thus, it makes sense to me that `LoopVectorizationCostModel::selectVectorizationFactor()` only checks `getForce()`

> Mmmh, I would have expected this to work the same way as `vectorize_width`.

So I don't see at this moment how it could work in the same way. And by working in the same way I think you mean adding `vectorize_predicate` checks to different places where `vectorize_width` is also checked, but that doesn't sound ideal to me.

Could a conclusion be that I was lucky? Lucky, because without all this knowledge that I have now, this patch looks to do what we would expect?


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

https://reviews.llvm.org/D65776