Skip to content
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

[DAGCombiner] Use generalized pattern matcher in foldBoolSelectToLogic #79101

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

ChunyuLiao
Copy link
Member

@ChunyuLiao ChunyuLiao commented Jan 23, 2024

support vp.select

TODO: Possibly other functions could be supported, eg: SimplifySelect()

@ChunyuLiao ChunyuLiao changed the title [RISCV] DAG combine special for AND/OR immediately [RISCV] DAG combine special case for AND/OR immediately Jan 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 23, 2024

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-risc-v

Author: Liao Chunyu (ChunyuLiao)

Changes

Combine:
RISCVISD::VMAND_VL x, 0/1, y -> 0/x
RISCVISD::VMXOR_VL x, 0/1, y -> x/1

td:
riscv_vmor_vl + riscv_vmand_vl + riscv_vmnot_vl,
better to select
vmorn.mm v0, v8, v0
instead of
vmand.mm v8, v8, v0
vmorn.mm v0, v8, v0


Full diff: https://github.com/llvm/llvm-project/pull/79101.diff

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+26)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td (+5)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll (+20)
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 76e8d21b818b259..cd8ced502e8bf3b 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -15621,6 +15621,32 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N,
     }
     break;
   }
+  case RISCVISD::VMAND_VL: {
+    SDValue N0 = N->getOperand(0);
+    SDValue N1 = N->getOperand(1);
+    if (isNullOrNullSplat(N0))
+      return N0;
+    else if (isNullOrNullSplat(N1))
+      return N1;
+    else if (isOneOrOneSplat(N0))
+      return N1;
+    else if (isOneOrOneSplat(N1))
+      return N0;
+    break;
+  }
+  case RISCVISD::VMOR_VL: {
+    SDValue N0 = N->getOperand(0);
+    SDValue N1 = N->getOperand(1);
+    if (isNullOrNullSplat(N1))
+      return N0;
+    else if (isNullOrNullSplat(N0))
+      return N1;
+    else if (isOneOrOneSplat(N0))
+      return N0;
+    else if (isOneOrOneSplat(N1))
+      return N1;
+    break;
+  }
   case ISD::SRA:
     if (SDValue V = performSRACombine(N, DAG, Subtarget))
       return V;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
index 6e7be2647e8f838..1f0a7d2e4fd752e 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
@@ -2736,6 +2736,11 @@ foreach mti = AllMasks in {
                                         VLOpFrag)),
               (!cast<Instruction>("PseudoVMANDN_MM_" # mti.LMul.MX)
                    VR:$rs1, VR:$rs2, GPR:$vl, mti.Log2SEW)>;
+    def : Pat<(mti.Mask (riscv_vmor_vl (riscv_vmand_vl VR:$rs1, VR:$rs2, VLOpFrag),
+                                       (riscv_vmnot_vl VR:$rs2, VLOpFrag),
+                                       VLOpFrag)),
+              (!cast<Instruction>("PseudoVMORN_MM_" # mti.LMul.MX)
+                   VR:$rs1, VR:$rs2, GPR:$vl, mti.Log2SEW)>;
     def : Pat<(mti.Mask (riscv_vmor_vl VR:$rs1,
                                        (riscv_vmnot_vl VR:$rs2, VLOpFrag),
                                        VLOpFrag)),
diff --git a/llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll b/llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll
index 9e7df5eab8dda98..72749368cc07e28 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/vselect-vp.ll
@@ -745,3 +745,23 @@ define <vscale x 16 x double> @select_nxv16f64(<vscale x 16 x i1> %a, <vscale x
   %v = call <vscale x 16 x double> @llvm.vp.select.nxv16f64(<vscale x 16 x i1> %a, <vscale x 16 x double> %b, <vscale x 16 x double> %c, i32 %evl)
   ret <vscale x 16 x double> %v
 }
+
+define <vscale x 2 x i1> @select_zero(<vscale x 2 x i1> %x, <vscale x 2 x i1> %y, <vscale x 2 x i1> %m, i32 zeroext %evl) {
+; CHECK-LABEL: select_zero:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli zero, a0, e8, mf4, ta, ma
+; CHECK-NEXT:    vmand.mm v0, v8, v0
+; CHECK-NEXT:    ret
+  %a = call <vscale x 2 x i1> @llvm.vp.select.nxv2i1(<vscale x 2 x i1> %x, <vscale x 2 x i1> %y, <vscale x 2 x i1> zeroinitializer, i32 %evl)
+  ret <vscale x 2 x i1> %a
+}
+
+define <vscale x 2 x i1> @select_one(<vscale x 2 x i1> %x, <vscale x 2 x i1> %y, <vscale x 2 x i1> %m, i32 zeroext %evl) {
+; CHECK-LABEL: select_one:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli zero, a0, e8, mf4, ta, ma
+; CHECK-NEXT:    vmorn.mm v0, v8, v0
+; CHECK-NEXT:    ret
+  %a = call <vscale x 2 x i1> @llvm.vp.select.nxv2i1(<vscale x 2 x i1> %x, <vscale x 2 x i1> %y, <vscale x 2 x i1> shufflevector (<vscale x 2 x i1> insertelement (<vscale x 2 x i1> undef, i1 true, i32 0), <vscale x 2 x i1> undef, <vscale x 2 x i32> zeroinitializer), i32 %evl)
+  ret <vscale x 2 x i1> %a
+}

@topperc
Copy link
Collaborator

topperc commented Jan 23, 2024

Isn't this a missed optimization on vp.select? Can we fix it there?

@ChunyuLiao ChunyuLiao changed the title [RISCV] DAG combine special case for AND/OR immediately [RISCV]DAG combine Special vp.select to logic Jan 24, 2024
; CHECK-NEXT: vsetvli zero, a0, e8, mf4, ta, ma
; CHECK-NEXT: vmor.mm v0, v0, v8
; CHECK-NEXT: ret
%a = call <vscale x 2 x i1> @llvm.vp.select.nxv2i1(<vscale x 2 x i1> %x, <vscale x 2 x i1> shufflevector (<vscale x 2 x i1> insertelement (<vscale x 2 x i1> undef, i1 true, i32 0), <vscale x 2 x i1> undef, <vscale x 2 x i32> zeroinitializer), <vscale x 2 x i1> %y, i32 %evl)
Copy link
Member Author

Choose a reason for hiding this comment

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

number on the left I'm not sure it's generic

  1. If it's not a generic case then the example can be removed
  2. If it is generic, then it can be optimized again

@ChunyuLiao ChunyuLiao changed the title [RISCV]DAG combine Special vp.select to logic [RISCV]DAG combine special vp.select to logic Jan 24, 2024
}
}
return SDValue();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to make this target agnostic and do this in DAGCombiner.cpp?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or in InstCombine? I remember that we have plan to extend InstCombine to handle VP intrinsics: https://llvm.org/docs/Proposals/VectorPredication.html#lift-instsimplify-instcombine-dagcombiner-to-vp

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or in InstCombine? I remember that we have plan to extend InstCombine to handle VP intrinsics: https://llvm.org/docs/Proposals/VectorPredication.html#lift-instsimplify-instcombine-dagcombiner-to-vp

We can't do it in InstCombine. Select blocks poison propagation, but and/or does not.

SelectionDAG is kind of bad about handling poison correctly. It already does this transform for ISD::VSELECT so doing it for VP_SELECT isn't any worse.

@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Jan 25, 2024
@ChunyuLiao ChunyuLiao changed the title [RISCV]DAG combine special vp.select to logic [DAGCombiner] Use generalized pattern matcher in foldBoolSelectToLogic Jan 25, 2024
@@ -12138,6 +12144,13 @@ SDValue DAGCombiner::foldVSelectOfConstants(SDNode *N) {
return SDValue();
}

SDValue DAGCombiner::visitVP_SELECT(SDNode *N) {
if (SDValue V = foldBoolSelectToLogic<EmptyMatchContext>(N, DAG))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be VPMatchContext?

define <vscale x 2 x i1> @select_zero(<vscale x 2 x i1> %x, <vscale x 2 x i1> %y, <vscale x 2 x i1> %m, i32 zeroext %evl) {
; CHECK-LABEL: select_zero:
; CHECK: # %bb.0:
; CHECK-NEXT: vsetvli a0, zero, e8, mf4, ta, ma
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem to be applying the VL

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

LGTM

support vp.select

TODO: Possibly other functions could be supported, eg: SimplifySelect()
@ChunyuLiao ChunyuLiao merged commit 45188c6 into llvm:main Jan 30, 2024
3 of 4 checks passed
@ChunyuLiao ChunyuLiao deleted the combine_VMAND_VL branch January 30, 2024 02:27
arsenm pushed a commit that referenced this pull request Apr 3, 2024
Hi all,

This patch is a follow-up of #79101. It migrates logic from
`visitVSELECT` to `visitVP_SELECT` to simplify `vp.select`. With this
patch we can do the following combinations:

```
vp.select undef, T, F --> T (if T is a constant), F otherwise
vp.select <condition>, undef, F --> F
vp.select <condition>, T, undef --> T
vp.select false, T, F --> F
vp.select <condition>, T, T --> T
```

I'm a total newbie to llvm and I'm sure there's room for improvements in
this patch. Please let me know if you have any advice. Thank you in
advance!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants