Page MenuHomePhabricator

[LV][Metadata] Add loop.interleave.enable for loop vectorizer
Needs ReviewPublic

Authored by eopXD on Sep 27 2022, 8:45 AM.

Details

Summary

Adding this metadata allows {loop.vectorize.enable, false} to be used
without disabling the whole pass.

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::lljit-initialize-deinitialize.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/lli -jit-kind=orc -jit-linker=jitlink -orc-runtime=/var/lib/buildkite-agent/builds/llvm-project/build/./lib/clang/16.0.0/lib/x86_64-unknown-linux-gnu/liborc_rt.a /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/lljit-initialize-deinitialize.ll | FileCheck /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/lljit-initialize-deinitialize.ll
170 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::priority-static-initializer.S
Script: -- : 'RUN: at line 4'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/priority-static-initializer.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/priority-static-initializer.S
160 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-cxa-atexit.S
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-cxa-atexit.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-cxa-atexit.S

Event Timeline

eopXD created this revision.Sep 27 2022, 8:45 AM
eopXD requested review of this revision.Sep 27 2022, 8:45 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptSep 27 2022, 8:45 AM
eopXD added a comment.Sep 27 2022, 8:46 AM

Please feel free to add appropriate reviewer to this patch.

mcberg2021 added inline comments.Sep 27 2022, 9:26 AM
clang/lib/CodeGen/CGLoopInfo.cpp
675

Can you update the comments on lines: 665 and 671 as they both need update.

eopXD updated this revision to Diff 463261.Sep 27 2022, 9:37 AM

Update comment.

eopXD marked an inline comment as done.EditedSep 27 2022, 9:40 AM

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.

eopXD updated this revision to Diff 463299.Sep 27 2022, 11:51 AM

Fix grammar error in comment.

eopXD added a comment.Oct 6 2022, 1:16 AM

Gentle ping, thank you ;)

fhahn added a comment.Oct 6 2022, 1:31 AM

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?

eopXD added a comment.Oct 6 2022, 1:41 AM

@fhahn Thank you very much for the review, backward compatibility is a good point. I will update the revision.

eopXD added a comment.Oct 6 2022, 8:32 AM

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