Adding this metadata allows {loop.vectorize.enable, false} to be used
without disabling the whole pass.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
clang/lib/CodeGen/CGLoopInfo.cpp | ||
---|---|---|
675 | Can you update the comments on lines: 665 and 671 as they both need update. |
After this patch, we can further remove setVectorizeWidth(1) and setInterleaveCount(1) when vectorize(disable) and interleave(disable) is specified. For now it is not removed to keep the patch limited and simple enough.
Adding this metadata allows {loop.vectorize.enable, false} to be used without disabling the whole pass.
Could you please describe the behavior in more detail here? The new metadata should also be documented in LangRef, the new pragma in https://clang.llvm.org/docs/LanguageExtensions.htm.
Will this change the existing behavior if only llvm.loop.vectorize.enable == false? To preserve backwards compatibility, I think the behavior shouldn;t change unless llvm.loop.interleave.enable is provided.
Also, shouldn't #pragma clang loop vectorize(disable) interleave(enable) be equivalent to #pragma clang loop vectorize(enable) vectorize_width(1)?
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
2142 | Will getInterleaveForce() return ENABLED if an interleave count > 1 is set through metadata? | |
3013 | Should this check interleaving or vectorization forced? |
@fhahn Thank you very much for the review, backward compatibility is a good point. I will update the revision.
Hi @fhahn,
After a second thought, I think posting a clear RFC [0] to the community that defines the
problem and goal this patch tries to achieve so the community is notified of such change
is a better approach than directly updating this patch. Please consider to drop by and
give a review.
Thank you very much for your time.
[0] [[RFC] Enabling LoopVectorizer for vectorization width of 1](https://discourse.llvm.org/t/rfc-enabling-loopvectorizer-for-vectorization-width-of-1/65769)
Regards,
eop Chen
Can you update the comments on lines: 665 and 671 as they both need update.