Skip to content

[RISCV][VLOPT] Don't reduce the VL is the same as CommonVL #123878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,12 @@ bool RISCVVLOptimizer::tryReduceVL(MachineInstr &MI) {
return false;
}

if (CommonVL->isIdenticalTo(VLOp)) {
LLVM_DEBUG(
dbgs() << " Abort due to CommonVL == VLOp, no point in reducing.\n");
return false;
}

if (CommonVL->isImm()) {
LLVM_DEBUG(dbgs() << " Reduce VL from " << VLOp << " to "
<< CommonVL->getImm() << " for " << MI << "\n");
Expand Down
27 changes: 27 additions & 0 deletions llvm/test/CodeGen/RISCV/rvv/vlopt-same-vl.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; RUN: llc -mtriple=riscv64 -mattr=+v -riscv-enable-vl-optimizer \
; RUN: -verify-machineinstrs -debug-only=riscv-vl-optimizer -o - 2>&1 %s | FileCheck %s

; REQUIRES: asserts

; GitHub Issue #123862 provided a case where the riscv-vl-optimizer pass was
; very slow. It was found that that case benefited greatly from aborting due
; to CommonVL == VLOp. Adding the case provided in the issue would show up
; as a long running test instead of a test failure. We would likley have a hard
; time figuring if that case had a regression. So instead, we check this output
; which was responsible for speeding it up.

define <vscale x 4 x i32> @same_vl_imm(<vscale x 4 x i32> %passthru, <vscale x 4 x i32> %a, <vscale x 4 x i32> %b) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment which explains why you're checking for the particular debug output.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, honestly, this seems like borderline bad practice. I would be fine with this landing without a dedicated test since it is NFC and "only" improves compile time.

Copy link
Contributor Author

@michaelmaitland michaelmaitland Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let me know what you prefer. This test case is testing observable change.

; CHECK: User VL is: 4
; CHECK-NEXT: Abort due to CommonVL == VLOp, no point in reducing.
%v = call <vscale x 4 x i32> @llvm.riscv.vadd.nxv4i32.nxv4i32(<vscale x 4 x i32> poison, <vscale x 4 x i32> %a, <vscale x 4 x i32> %b, i64 4)
%w = call <vscale x 4 x i32> @llvm.riscv.vadd.nxv4i32.nxv4i32(<vscale x 4 x i32> poison, <vscale x 4 x i32> %v, <vscale x 4 x i32> %a, i64 4)
ret <vscale x 4 x i32> %w
}

define <vscale x 4 x i32> @same_vl_reg(<vscale x 4 x i32> %passthru, <vscale x 4 x i32> %a, <vscale x 4 x i32> %b, i64 %vl) {
; CHECK: User VL is: %3:gprnox0
; CHECK-NEXT: Abort due to CommonVL == VLOp, no point in reducing.
%v = call <vscale x 4 x i32> @llvm.riscv.vadd.nxv4i32.nxv4i32(<vscale x 4 x i32> poison, <vscale x 4 x i32> %a, <vscale x 4 x i32> %b, i64 %vl)
%w = call <vscale x 4 x i32> @llvm.riscv.vadd.nxv4i32.nxv4i32(<vscale x 4 x i32> poison, <vscale x 4 x i32> %v, <vscale x 4 x i32> %a, i64 %vl)
ret <vscale x 4 x i32> %w
}
Loading