[PATCH] D70485: [ARM,MVE] Add intrinsics to deal with predicates.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:780
 
 def int_arm_vctp8  : Intrinsic<[llvm_v16i1_ty], [llvm_i32_ty], [IntrNoMem]>;
 def int_arm_vctp16 : Intrinsic<[llvm_v8i1_ty], [llvm_i32_ty], [IntrNoMem]>;
----------------
Should we rename int_arm_vctp8 to int_arm_mve_vctp8?


================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:783
 def int_arm_vctp32 : Intrinsic<[llvm_v4i1_ty], [llvm_i32_ty], [IntrNoMem]>;
 def int_arm_vctp64 : Intrinsic<[llvm_v2i1_ty], [llvm_i32_ty], [IntrNoMem]>;
 
----------------
Is this ever used anywhere? It doesn't select as far as I can see, so I think you can just remove it.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:4202
 
-def MVE_VCTP8  : MVE_VCTP<"8",  0b00>;
-def MVE_VCTP16 : MVE_VCTP<"16", 0b01>;
-def MVE_VCTP32 : MVE_VCTP<"32", 0b10>;
-def MVE_VCTP64 : MVE_VCTP<"64", 0b11>;
+multiclass MVE_VCTP_and_patterns<MVEVectorVTInfo VTI, Intrinsic intr> {
+  def "": MVE_VCTP<VTI.BitsSuffix, VTI.Size>;
----------------
Maybe call this MVE_VCTP and name the instruction MVE_VCTPInst, or something like it.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:4206
+  let Predicates = [HasMVEInt] in {
+    def : Pat<(intr rGPR:$Rn), (VTI.Pred (!cast<Instruction>(NAME) rGPR:$Rn))>;
+    def : Pat<(and (intr rGPR:$Rn), (VTI.Pred VCCR:$mask)),
----------------
Two lines on this one please, just to make it more obvious when looking at it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70485/new/

https://reviews.llvm.org/D70485