[PATCH] D120449: [RISCV][RVV] Add strict vfcvt intrinsics that have side effects for dynamically-set rounding mode
Zakk Chen via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 4 08:01:58 PST 2022
khchen added a comment.
Makes sense to me, but I'd appreciate someone else for a final LGTM.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4683
+ Ops.push_back(VL);
+ Ops.push_back(DAG.getUNDEF(XLenVT)); // Policy
+ }
----------------
arcbbb wrote:
> kito-cheng wrote:
> > Is this operand for tail policy? if so why this is `UNDEF`? I guess this should be `TAIL_AGNOSTIC` rather than `UNDEF`?
> Because unmasked pseudos doesn't have a policy operand,
> TA/TU is distinguished by checking if passthru is undef. I think it is proper to leave undef here.
nit: maybe we could add a comment for that, because it must be `UNDEF` to match unmaksed pattern successfully.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4678
+ MVT MaskVT = MVT::getVectorVT(MVT::i1, VT.getVectorElementCount());
+ SDValue Mask = DAG.getNode(RISCVISD::VMSET_VL, DL, MaskVT, VL);
+ Ops.push_back(Mask);
----------------
I think `TrueMask` maybe better.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:619
+ VReg result_reg_class,
+ VReg op2_reg_class
+ > {
----------------
--> `VReg op2_reg_class> {`
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:649
+ defvar ivti = GetIntVTypeInfo<fvti>.Vti;
+
+ defm : VPatConvertStrictSDNode_VL<vop, instruction, "V",
----------------
This is additional newline comparing to `VPatConvertI2FPSDNode_V_VL_STRICT`? or we need a newline in `VPatConvertI2FPSDNode_V_VL_STRICT`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120449/new/
https://reviews.llvm.org/D120449
More information about the cfe-commits
mailing list