Page MenuHomePhabricator

[RISCV] Enable interleaved vectorization for RVV
ClosedPublic

Authored by luke957 on Apr 28 2021, 9:17 AM.

Details

Summary

Enable interleaved vectorization for RVV.

Diff Detail

Event Timeline

luke957 created this revision.Apr 28 2021, 9:17 AM
luke957 requested review of this revision.Apr 28 2021, 9:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 9:17 AM
craig.topper added inline comments.Apr 28 2021, 9:27 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
5

Why are we not checking the generated IR?

42

Do we need the debug info?

luke957 updated this revision to Diff 341536.Apr 29 2021, 8:55 AM
luke957 added inline comments.Apr 29 2021, 9:06 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
5

Checking the generated IR is better.

42

DI is not necessary.

Do you still need to update the diff to address the previous comments?

Do you still need to update the diff to address the previous comments?

Em, I have modified the test case according to the comments "checking the generated IR" and "debug info"(DI removed). Any other comments?

craig.topper added inline comments.May 18 2021, 10:09 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
7

Is this just checking the induction variable increment? I'd really like to see what vector instructions it generates.

luke957 updated this revision to Diff 346778.May 20 2021, 10:15 AM

Rebase and update.

luke957 added inline comments.May 20 2021, 10:27 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
7

For this test case, vf and uf will be 4 and 2 respectively. So the vector instructions will repeat once in one trip, and there will be an instruction like %{{.*}} = add <4 x i32> %{{.*}}, <i32 4, i32 4, i32 4, i32 4>.

This revision is now accepted and ready to land.Fri, May 28, 9:56 AM
This revision was automatically updated to reflect the committed changes.

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

Thanks for reminding me. Sorry for my carelessness.

I just noticed that this enabled interleaving in the loop vectorizer even when the V extension isn't enabled. So we now generate interleaved scalar code in some cases. Was that intentional?

Thanks for reminding me. Sorry for my carelessness.

Fixed in https://reviews.llvm.org/D103787

craig.topper added inline comments.Tue, Jun 8, 11:56 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
173

I think these are two different features.

enableInterleavedAccessVectorization is for memory accesses that are interlaved.

getMaxInterleaveFactor controls loop unrolling in the vectorizer.

Which feature were you trying to enable?

craig.topper added inline comments.Tue, Jun 8, 11:57 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
173

I think enableInterleavedAccessVectorization reads extra data and uses shuffles to extract the elements that are needed.

jrtc27 added inline comments.Tue, Jun 8, 12:45 PM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
5

Why is this not using update_test_checks.py?

11–50

This IR is very messy, Clang-output IR does not always make for clean test cases. We don't need Function Attrs comments, we don't need press comments, many of the attributes are unnecessary, and the ones that are are best done inline. IR tests should be minimal, ideally from-scratch, but whittling Clang-produced IR down to something that could feasibly have been hand-written (or generated by a simple tool, like RVV and RVA tests) is fine.

luke957 added inline comments.Wed, Jun 16, 1:21 AM
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h
173

Yes, bool enableInterleavedAccessVectorization() should not be added here. I'll restore the code and submit a new patch.

luke957 added inline comments.Wed, Jun 16, 2:24 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
5

Yeah, using update_test_checks.py is better. But as said update_test_checks.py itself, update_test_checks.py is not designed to be authoritative about what constitutes a good test :)

11–50

Thanks for the review. I'll update the test case to look canonical.

luke957 added inline comments.Wed, Jun 16, 2:29 AM
llvm/test/Transforms/LoopVectorize/RISCV/riscv-interleaved.ll
5

Thanks for the comment. I'll try to update the test case using update_test_checks.py.