[PATCH] D146752: [RISCV][RISCVISelLowering] Add tail agnostic policy operand to VECREDUCE instructions

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:6287
+  SDValue Policy = DAG.getTargetConstant(RISCVII::TAIL_AGNOSTIC, DL, XLenVT);
+  SDValue Ops[6] = {PassThru, Vec, InitialValue, Mask, VL, Policy};
+  SDValue Reduction = DAG.getNode(RVVOpcode, DL, M1VT, Ops);
----------------
Remove the 6 from the brackets. Let the compiler autodetect the size.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:8704
 
+  SDValue Ops[6] = {Reduce.getOperand(0), Reduce.getOperand(1),
+                    NewScalarV,           Reduce.getOperand(3),
----------------
Same here


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:4887
+  def : VPatTernaryNoMaskTA<intrinsic, inst, kind, result_type, op1_type,
+                                    op2_type, sew, vlmul, result_reg_class,
+                                    op1_reg_class, op2_kind>;
----------------
Reduce indentation to line `op2_type` with `intrinsic` from previous line


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:4890
+  def : VPatTernaryMaskTA<intrinsic, inst, kind, result_type, op1_type, op2_type,
+                              mask_type, sew, vlmul, result_reg_class, op1_reg_class,
+                              op2_kind>;
----------------
Same here


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:4999
+    defm : VPatTernaryTA<intrinsic, instruction, "VS",
                        vectorM1.Vector, vti.Vector,
                        vectorM1.Vector, vti.Mask,
----------------
Same here


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:5007
+    defm : VPatTernaryTA<intrinsic, instruction, "VS",
                        gvti.VectorM1, gvti.Vector,
                        gvti.VectorM1, gvti.Mask,
----------------
Indent 2 more spaces


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:5021
+      defm : VPatTernaryTA<intrinsic, instruction, "VS",
                          wtiM1.Vector, vti.Vector,
                          wtiM1.Vector, vti.Mask,
----------------
Same here


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:1998
 let Predicates = [HasVInstructions] in {
-defm : VPatReductionVL<rvv_vecreduce_ADD_vl,  "PseudoVREDSUM", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_UMAX_vl, "PseudoVREDMAXU", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_SMAX_vl, "PseudoVREDMAX", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_UMIN_vl, "PseudoVREDMINU", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_SMIN_vl, "PseudoVREDMIN", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_AND_vl,  "PseudoVREDAND", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_OR_vl,   "PseudoVREDOR", /*is_float*/0>;
-defm : VPatReductionVL<rvv_vecreduce_XOR_vl,  "PseudoVREDXOR", /*is_float*/0>;
+defm : VPatReductionVL_Policy<rvv_vecreduce_ADD_vl,  "PseudoVREDSUM", /*is_float*/0>;
+defm : VPatReductionVL_Policy<rvv_vecreduce_UMAX_vl, "PseudoVREDMAXU", /*is_float*/0>;
----------------
ARe there still users of the old `VPatReductionVL` class?


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-reduction-fp-vp.ll:14
 ; CHECK-NEXT:    vfmv.s.f v9, fa0
-; CHECK-NEXT:    vsetvli zero, a0, e16, mf4, tu, ma
+; CHECK-NEXT:    vsetvli zero, a0, e16, mf4, ta, mu
 ; CHECK-NEXT:    vfredusum.vs v9, v8, v9, v0.t
----------------
We should not be using `mu` here. It appears the switch to `VPseudoTernaryWithPolicy` caused us to get `let UsesMaskPolicy=1` for these instructions. `VPseudoTernary` used `VPseudoBinaryMask` which does not set the `UsesMaskPolicy=1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146752