-
Notifications
You must be signed in to change notification settings - Fork 11.7k
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] Check vmerge's true is in same block in vmerge -> vmv.v.v peephole #110861
[RISCV] Check vmerge's true is in same block in vmerge -> vmv.v.v peephole #110861
Conversation
…phole The peepholes in RISCVVectorPeephole need to be local and we were failing to check if the true operand was in the same block as the vmerge. Fixes llvm#110832
@llvm/pr-subscribers-backend-risc-v Author: Luke Lau (lukel97) ChangesThe peepholes in RISCVVectorPeephole need to be local and we were failing to check if the true operand was in the same block as the vmerge. Fixes #110832 Full diff: https://github.com/llvm/llvm-project/pull/110861.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
index 026e5d653c38c2..b883c50beadc09 100644
--- a/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp
@@ -419,8 +419,8 @@ bool RISCVVectorPeephole::convertSameMaskVMergeToVMv(MachineInstr &MI) {
if (!NewOpc)
return false;
MachineInstr *True = MRI->getVRegDef(MI.getOperand(3).getReg());
- if (!True || !RISCV::getMaskedPseudoInfo(True->getOpcode()) ||
- !hasSameEEW(MI, *True))
+ if (!True || True->getParent() != MI.getParent() ||
+ !RISCV::getMaskedPseudoInfo(True->getOpcode()) || !hasSameEEW(MI, *True))
return false;
const MachineInstr *TrueV0Def = V0Defs.lookup(True);
diff --git a/llvm/test/CodeGen/RISCV/rvv/rvv-peephole-vmerge-to-vmv.mir b/llvm/test/CodeGen/RISCV/rvv/rvv-peephole-vmerge-to-vmv.mir
index e2f1fe4094b5c9..a5622fd466217c 100644
--- a/llvm/test/CodeGen/RISCV/rvv/rvv-peephole-vmerge-to-vmv.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/rvv-peephole-vmerge-to-vmv.mir
@@ -159,3 +159,29 @@ body: |
$v0 = COPY %mask
%x:vrnov0 = PseudoVMERGE_VVM_M1 $noreg, %false, %true, $v0, 4, 5 /* e32 */
...
+---
+# Shouldn't be converted because true is in a different block
+name: same_mask_diff_blocks
+body: |
+ ; CHECK-LABEL: name: same_mask_diff_blocks
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $v8, $v0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %false:vr = COPY $v8
+ ; CHECK-NEXT: %mask:vr = COPY $v0
+ ; CHECK-NEXT: $v0 = COPY %mask
+ ; CHECK-NEXT: %true:vrnov0 = PseudoVADD_VV_M1_MASK $noreg, $noreg, $noreg, $v0, 4, 5 /* e32 */, 0 /* tu, mu */
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: $v0 = COPY %mask
+ ; CHECK-NEXT: [[PseudoVMERGE_VVM_M1_:%[0-9]+]]:vrnov0 = PseudoVMERGE_VVM_M1 $noreg, %false, %true, $v0, 4, 5 /* e32 */
+ bb.0:
+ liveins: $v8, $v0
+ %false:vr = COPY $v8
+ %mask:vr = COPY $v0
+ $v0 = COPY %mask
+ %true:vrnov0 = PseudoVADD_VV_M1_MASK $noreg, $noreg, $noreg, $v0, 4, 5 /* e32 */, 0 /* tu, mu */
+ bb.1:
+ $v0 = COPY %mask
+ %5:vrnov0 = PseudoVMERGE_VVM_M1 $noreg, %false, %true, $v0, 4, 5 /* e32 */
|
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.
Thanks for the quick action, have run this against my original non-reduced testcase and can confirm that also works with your PR.
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
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/134/builds/6390 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/4853 Here is the relevant piece of the build log for the reference
|
The peepholes in RISCVVectorPeephole need to be local and we were failing to check if the true operand was in the same block as the vmerge.
Fixes #110832