-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV] Don't move source if passthru already dominates in vmv.v.v peephole #105792
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
[RISCV] Don't move source if passthru already dominates in vmv.v.v peephole #105792
Conversation
…ephole Currently we move the source down to where vmv.v.v to make sure that the new passthru dominates, but we do this even if it already does. This adds a simple local dominance check (taken from X86FastPreTileConfig.cpp) and avoids doing the move if it can. It also modifies the move to only move it to just past the passthru definition, and not all the way down to the vmv.v.v. This allows folding to succeed in some edge cases, which prevents regressions in an upcoming patch.
@llvm/pr-subscribers-backend-risc-v Author: Luke Lau (lukel97) ChangesCurrently we move the source down to where vmv.v.v to make sure that the new passthru dominates, but we do this even if it already does. This adds a simple local dominance check (taken from X86FastPreTileConfig.cpp) and avoids doing the move if it can. It also modifies the move to only move it to just past the passthru definition, and not all the way down to the vmv.v.v. This allows folding to succeed in some edge cases, which prevents regressions in an upcoming patch. Full diff: https://github.com/llvm/llvm-project/pull/105792.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
index 9772782ad3d6db..822ab492c710b4 100644
--- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
@@ -437,6 +437,22 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) {
return From.isSafeToMove(SawStore);
}
+/// Given A and B are in the same MBB, returns true if A comes before B.
+static bool dominates(MachineBasicBlock::const_iterator A,
+ MachineBasicBlock::const_iterator B) {
+ assert(A->getParent() == B->getParent());
+ const MachineBasicBlock *MBB = A->getParent();
+ auto MBBEnd = MBB->end();
+ if (B == MBBEnd)
+ return true;
+
+ MachineBasicBlock::const_iterator I = MBB->begin();
+ for (; &*I != A && &*I != B; ++I)
+ ;
+
+ return &*I == A;
+}
+
/// If a PseudoVMV_V_V is the only user of its input, fold its passthru and VL
/// into it.
///
@@ -481,12 +497,15 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) {
if (!isVLKnownLE(SrcVL, MI.getOperand(3)))
return false;
- // If Src ends up using MI's passthru/VL, move it so it can access it.
- // TODO: We don't need to do this if they already dominate Src.
- if (!SrcPassthru.isIdenticalTo(Passthru)) {
- if (!isSafeToMove(*Src, MI))
- return false;
- Src->moveBefore(&MI);
+ // If the new passthru doesn't dominate Src, try to move Src so it does.
+ if (Passthru.getReg() != RISCV::NoRegister) {
+ MachineInstr *PassthruDef = MRI->getVRegDef(Passthru.getReg());
+ if (PassthruDef->getParent() == Src->getParent() &&
+ !dominates(PassthruDef, Src)) {
+ if (!isSafeToMove(*Src, *PassthruDef->getNextNode()))
+ return false;
+ Src->moveBefore(PassthruDef->getNextNode());
+ }
}
if (SrcPassthru.getReg() != Passthru.getReg()) {
diff --git a/llvm/test/CodeGen/RISCV/rvv/vmv.v.v-peephole.mir b/llvm/test/CodeGen/RISCV/rvv/vmv.v.v-peephole.mir
new file mode 100644
index 00000000000000..b2526c6df6939e
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/vmv.v.v-peephole.mir
@@ -0,0 +1,20 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+# RUN: llc %s -o - -mtriple=riscv64 -mattr=+v -run-pass=riscv-vector-peephole \
+# RUN: -verify-machineinstrs | FileCheck %s
+
+---
+name: move_src
+body: |
+ bb.0:
+ liveins: $v8
+ ; CHECK-LABEL: name: move_src
+ ; CHECK: liveins: $v8
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %passthru:vr = COPY $v8
+ ; CHECK-NEXT: %x:vr = PseudoVADD_VV_M1 %passthru, $noreg, $noreg, 4, 5 /* e32 */, 0 /* tu, mu */
+ ; CHECK-NEXT: %y:gpr = ADDI $x0, 1
+ %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 4, 5 /* e32 */, 0 /* tu, mu */
+ %passthru:vr = COPY $v8
+ %y:gpr = ADDI $x0, 1
+ %z:vr = PseudoVMV_V_V_M1 %passthru, %x, 4, 5 /* e32 */, 0 /* tu, mu */
+...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…3baa081ca Local branch amd-gfx fee3baa Merged main:99b85cae628c1cc5641944290712cd84ccf1f6c8 into amd-gfx:f09ee8740921 Remote branch main be5ecc3 [RISCV] Dont move source if passthru already dominates in vmv.v.v peephole (llvm#105792)
Currently we move the source down to where vmv.v.v to make sure that the new passthru dominates, but we do this even if it already does.
This adds a simple local dominance check (taken from X86FastPreTileConfig.cpp) and avoids doing the move if it can.
It also modifies the move to only move it to just past the passthru definition, and not all the way down to the vmv.v.v.
This allows folding to succeed in some edge cases, which prevents regressions in an upcoming patch.