With this change, the VPlan native path is triggered with the directive:
#pragma clang loop vectorize(enable)
There is no need to specify the vectorize_width(N) clause.
Differential D57598
[VPLAN] Determine Vector Width programmatically. fpetrogalli on Feb 1 2019, 9:51 AM. Authored by
Details With this change, the VPlan native path is triggered with the directive: #pragma clang loop vectorize(enable) There is no need to specify the vectorize_width(N) clause.
Diff Detail Event TimelineComment Actions While we are at this, let's talk about downstream dependency, if any, for allowing more than one candidate VF along the native path. At least we can write down a list of TODOs so that we'll be aware of the things we need to improve at a time in the future. This may be a bit more than what you anticipated for this patch, but I think this will help everyone.
Comment Actions Thanks for working on this Francesco! +1 to Hideki's point about moving to LoopVectorizationPlanner.
Comment Actions @hsaito , @fhahn , @npanchen , Thank you for looking into this. I have responded to all comments about the design. I will work on a new patch an submit it according to your suggestions. For now, I have ignored those comments on trivial changes (for example, @fhahn comment on adding more test coverage). I will fix such comments and update them along the way with the re-implementation of the patch. Thank you, Francesco
Comment Actions Thanks Francesco for helping us remove some of the constraints we have in VPlan native path! One more comment below.
Comment Actions Hi all, I finally addressed your comments and update the patch. Let me know what you think. Francesco
Comment Actions I mostly changes to code to use the infrastructure that LLVM already provides to determine the vectorization factor. Comment Actions Haven't gone through the LIT test yet, but the code addressed my concerns.
Comment Actions LGTM. Please wait for a few days to give others a chance to go over your updated patch.
Comment Actions I think you waited long enough already. I suggest proceeding to commit. Any further comments can be addressed post-commit. Comment Actions
OK. @hsaito, I don't have commit access, could you commit this change for me? Thank you! Comment Actions
Will do. I had a day off today. Will take care of it, hopefully tomorrow.
Comment Actions I forgot to update the comment in the test as requested from @fhahn . Now it is done.
Comment Actions @fhahn, I'm trying to figure out the appropriate proxy setting for external git. |
Is this related to the patch? I suppose guessVectorizationFactor could set UserVF to 1, which could reach this code. It would be better to not vectorize with VF == 1.