This patch adds functionality 'llvm.loop.aligned' metadata. The load/store accesses are aligned to 32 bit for loops marked with 'llvm.loop.aligned'.
This is used by D69897
Details
- Reviewers
hfinkel Meinersbur DTharun rscottmanley
Diff Detail
Event Timeline
This seems to be only be used by the vectorizer. Are there plans for meanings beyond LoopVectorize? If not, it should be reflected in the metadata name: llvm.loop.vectorize.aligned.enable.
The semantics just assume that the data is aligned, but does not make it aligned. This should be reflected in the metadata name.
I do not see why an alignment of 32 fits everybody. Why not making it configurable, e.g. { !"llvm.loop.vectorize.assume_alignment", i64 32 }?
A regression test is missing.
llvm/docs/LangRef.rst | ||
---|---|---|
5443 | Unrelated? Also, I think that reStructured Spinx warns if the underline is not long enough, and the bots are configured with fail-on-warnings. | |
llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp | ||
207–208 | [nit] Please complete remove commented source | |
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
806 | [nit] unrelated | |
7316 | Since this function is only used withing this translation unit, make it static. Also add a doxygen comment explaining what this function does. | |
7316–7317 | [style] Please run clang-format over your code (git clang-format HEAD) | |
7322 | [style] The LLVM coding standard does not use almost-always-auto. | |
7328–7329 | Please completely remove commented source |
Do we need this at all? Can't we just emit @llvm.assume calls before the loop if we know the alignment? I've done this in the past and it's worked well (by using __builtin_assume_aligned from Clang).
I'm in favor of doing this (llvm.assume) in the frontedn so other passes can use the information as well without requiring them to understand yet another encoding.
Unrelated?
Also, I think that reStructured Spinx warns if the underline is not long enough, and the bots are configured with fail-on-warnings.