[llvm] [RISCV] Ensure false dominates in vmerge peephole (PR #181664)
Luke Lau via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 16 22:07:52 PST 2026
https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/181664
>From 501f71d203a8632de265197c850c647a936b8f1d Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 16 Feb 2026 20:14:23 +0800
Subject: [PATCH 1/3] Precommit tests
---
llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir b/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
index d24b87a1d2619..98cfc6a8fefb3 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
@@ -243,3 +243,16 @@ body: |
%load:vrnov0 = PseudoVLE32_V_M1 $noreg, $noreg, 1, 5 /* e32 */, 0 /* tu, mu */ :: (load unknown-size)
%merge:vrnov0 = PseudoVMERGE_VVM_M1 %passthru, %passthru, %load, %mask, %avl, 5 /* e32 */
...
+---
+name: sink_past_false
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: sink_past_false
+ ; CHECK: %mask:vmv0 = COPY $v0
+ ; CHECK-NEXT: %z:vrnov0 = PseudoVADD_VV_M1_MASK %y, $noreg, $noreg, %mask, -1 /* vl=VLMAX */, 5 /* e32 */, 1 /* ta, mu */
+ ; CHECK-NEXT: %y:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1 /* vl=VLMAX */, 5 /* e32 */, 3 /* ta, ma */
+ %mask:vmv0 = COPY $v0
+ %x:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 5, 3
+ %y:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 5, 3
+ %z:vrnov0 = PseudoVMERGE_VVM_M1 $noreg, %y:vrnov0, %x:vrnov0, %mask, -1, 5
+...
>From 5448ff69447abdc79a33889befd970c955b656e8 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 16 Feb 2026 20:14:46 +0800
Subject: [PATCH 2/3] [RISCV] Ensure false dominates in vmerge peephole
When folding vmerge into it's true operand, true will eventually use the false operand as its passthru, but we don't check that the instruction defining false dominates true. This can cause a use before def.
Fix this by sinking true past false. We already do this for the mask, so this does it in the same call to ensureDominates.
We don't seem to run into this with current codegen but upcoming changes to RISCVVLOptimizer expose it.
---
llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 49 ++++++++++++++-----
.../CodeGen/RISCV/rvv/vmerge-peephole.mir | 2 +-
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
index eea350b1d1725..28e56d8ee719b 100644
--- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
@@ -73,6 +73,7 @@ class RISCVVectorPeephole : public MachineFunctionPass {
bool isAllOnesMask(const MachineInstr *MaskDef) const;
std::optional<unsigned> getConstant(const MachineOperand &VL) const;
bool ensureDominates(const MachineOperand &Use, MachineInstr &Src) const;
+ bool ensureDominates(MachineInstr *Def, MachineInstr &Src) const;
Register
lookThruCopies(Register Reg, bool OneUseOnly = false,
SmallVectorImpl<MachineInstr *> *Copies = nullptr) const;
@@ -558,20 +559,34 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) {
return From.isSafeToMove(SawStore);
}
+/// Given \p A and \p B are in the same block, returns the instruction that
+/// comes first.
+static MachineBasicBlock::iterator first(MachineBasicBlock::iterator A,
+ MachineBasicBlock::iterator B) {
+ assert(A->getParent() == B->getParent());
+ MachineBasicBlock::iterator I = A->getParent()->begin();
+ for (; &*I != A && &*I != B; ++I)
+ ;
+ return I;
+}
+
+/// Given \p A and \p B are in the same block, returns the instruction that
+/// comes last.
+static MachineBasicBlock::iterator last(MachineBasicBlock::iterator A,
+ MachineBasicBlock::iterator B) {
+ return first(A, B) == A ? B : A;
+}
+
/// 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) {
+static bool dominates(MachineBasicBlock::iterator A,
+ MachineBasicBlock::iterator B) {
assert(A->getParent() == B->getParent());
- const MachineBasicBlock *MBB = A->getParent();
+ 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;
+ return first(A, B) == A;
}
/// If the register in \p MO doesn't dominate \p Src, try to move \p Src so it
@@ -583,7 +598,11 @@ bool RISCVVectorPeephole::ensureDominates(const MachineOperand &MO,
if (!MO.isReg() || !MO.getReg().isValid())
return true;
- MachineInstr *Def = MRI->getVRegDef(MO.getReg());
+ return ensureDominates(MRI->getVRegDef(MO.getReg()), Src);
+}
+
+bool RISCVVectorPeephole::ensureDominates(MachineInstr *Def,
+ MachineInstr &Src) const {
if (Def->getParent() == Src.getParent() && !dominates(Def, Src)) {
if (!isSafeToMove(Src, *Def->getNextNode()))
return false;
@@ -832,10 +851,14 @@ bool RISCVVectorPeephole::foldVMergeToMask(MachineInstr &MI) const {
assert(RISCVII::hasVecPolicyOp(True.getDesc().TSFlags) &&
"Foldable unmasked pseudo should have a policy op already");
- // Make sure the mask dominates True and its copies, otherwise move down True
- // so it does. VL will always dominate since if it's a register they need to
- // be the same.
- if (!ensureDominates(MaskOp, True))
+ // Make sure both mask and false dominate True and its copies, otherwise move
+ // down True so it does. VL will always dominate since if it's a register they
+ // need to be the same.
+ MachineInstr *False = MRI->getUniqueVRegDef(FalseReg);
+ MachineInstr *LastDef = Mask;
+ if (False && False->getParent() == Mask->getParent())
+ LastDef = &*last(LastDef, False);
+ if (!ensureDominates(LastDef, True))
return false;
if (NeedsCommute) {
diff --git a/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir b/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
index 98cfc6a8fefb3..3f4ff342265d4 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vmerge-peephole.mir
@@ -249,8 +249,8 @@ body: |
bb.0:
; CHECK-LABEL: name: sink_past_false
; CHECK: %mask:vmv0 = COPY $v0
- ; CHECK-NEXT: %z:vrnov0 = PseudoVADD_VV_M1_MASK %y, $noreg, $noreg, %mask, -1 /* vl=VLMAX */, 5 /* e32 */, 1 /* ta, mu */
; CHECK-NEXT: %y:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1 /* vl=VLMAX */, 5 /* e32 */, 3 /* ta, ma */
+ ; CHECK-NEXT: %z:vrnov0 = PseudoVADD_VV_M1_MASK %y, $noreg, $noreg, %mask, -1 /* vl=VLMAX */, 5 /* e32 */, 1 /* ta, mu */
%mask:vmv0 = COPY $v0
%x:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 5, 3
%y:vrnov0 = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 5, 3
>From 20c63d8d7ba482cfc2c5c8af479fbd97b21308f6 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 17 Feb 2026 14:07:19 +0800
Subject: [PATCH 3/3] Remove ensureDominates overload
---
llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp | 49 ++++++-------------
1 file changed, 16 insertions(+), 33 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
index 28e56d8ee719b..c7c8eeed02aeb 100644
--- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
@@ -73,7 +73,6 @@ class RISCVVectorPeephole : public MachineFunctionPass {
bool isAllOnesMask(const MachineInstr *MaskDef) const;
std::optional<unsigned> getConstant(const MachineOperand &VL) const;
bool ensureDominates(const MachineOperand &Use, MachineInstr &Src) const;
- bool ensureDominates(MachineInstr *Def, MachineInstr &Src) const;
Register
lookThruCopies(Register Reg, bool OneUseOnly = false,
SmallVectorImpl<MachineInstr *> *Copies = nullptr) const;
@@ -559,34 +558,20 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) {
return From.isSafeToMove(SawStore);
}
-/// Given \p A and \p B are in the same block, returns the instruction that
-/// comes first.
-static MachineBasicBlock::iterator first(MachineBasicBlock::iterator A,
- MachineBasicBlock::iterator B) {
- assert(A->getParent() == B->getParent());
- MachineBasicBlock::iterator I = A->getParent()->begin();
- for (; &*I != A && &*I != B; ++I)
- ;
- return I;
-}
-
-/// Given \p A and \p B are in the same block, returns the instruction that
-/// comes last.
-static MachineBasicBlock::iterator last(MachineBasicBlock::iterator A,
- MachineBasicBlock::iterator B) {
- return first(A, B) == A ? B : A;
-}
-
/// Given A and B are in the same MBB, returns true if A comes before B.
-static bool dominates(MachineBasicBlock::iterator A,
- MachineBasicBlock::iterator B) {
+static bool dominates(MachineBasicBlock::const_iterator A,
+ MachineBasicBlock::const_iterator B) {
assert(A->getParent() == B->getParent());
- MachineBasicBlock *MBB = A->getParent();
+ const MachineBasicBlock *MBB = A->getParent();
auto MBBEnd = MBB->end();
if (B == MBBEnd)
return true;
- return first(A, B) == A;
+ MachineBasicBlock::const_iterator I = MBB->begin();
+ for (; &*I != A && &*I != B; ++I)
+ ;
+
+ return &*I == A;
}
/// If the register in \p MO doesn't dominate \p Src, try to move \p Src so it
@@ -598,11 +583,7 @@ bool RISCVVectorPeephole::ensureDominates(const MachineOperand &MO,
if (!MO.isReg() || !MO.getReg().isValid())
return true;
- return ensureDominates(MRI->getVRegDef(MO.getReg()), Src);
-}
-
-bool RISCVVectorPeephole::ensureDominates(MachineInstr *Def,
- MachineInstr &Src) const {
+ MachineInstr *Def = MRI->getVRegDef(MO.getReg());
if (Def->getParent() == Src.getParent() && !dominates(Def, Src)) {
if (!isSafeToMove(Src, *Def->getNextNode()))
return false;
@@ -762,7 +743,8 @@ bool RISCVVectorPeephole::foldVMergeToMask(MachineInstr &MI) const {
// Collect chain of COPYs on True's result for later cleanup.
SmallVector<MachineInstr *, 4> TrueCopies;
Register PassthruReg = lookThruCopies(MI.getOperand(1).getReg());
- Register FalseReg = lookThruCopies(MI.getOperand(2).getReg());
+ const MachineOperand &FalseOp = MI.getOperand(2);
+ Register FalseReg = lookThruCopies(FalseOp.getReg());
Register TrueReg = lookThruCopies(MI.getOperand(3).getReg(),
/*OneUseOnly=*/true, &TrueCopies);
if (!TrueReg.isVirtual() || !MRI->hasOneUse(TrueReg))
@@ -854,11 +836,12 @@ bool RISCVVectorPeephole::foldVMergeToMask(MachineInstr &MI) const {
// Make sure both mask and false dominate True and its copies, otherwise move
// down True so it does. VL will always dominate since if it's a register they
// need to be the same.
+ const MachineOperand *DomOp = &MaskOp;
MachineInstr *False = MRI->getUniqueVRegDef(FalseReg);
- MachineInstr *LastDef = Mask;
- if (False && False->getParent() == Mask->getParent())
- LastDef = &*last(LastDef, False);
- if (!ensureDominates(LastDef, True))
+ if (False && False->getParent() == Mask->getParent() &&
+ dominates(Mask, False))
+ DomOp = &FalseOp;
+ if (!ensureDominates(*DomOp, True))
return false;
if (NeedsCommute) {
More information about the llvm-commits
mailing list