LuoYuanke added inline comments.
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:1955
setOperationAction(ISD::SCALAR_TO_VECTOR, MVT::v32f16, Custom);
+ setOperationAction(ISD::SINT_TO_FP, MVT::v32i16, Legal);
+ setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::v32i16, Legal);
----------------
Sorry, I'm just confused on why the type is the same for ISD::SINT_TO_FP and
ISD::FP_TO_SINT? The legalization use src type for ISD::SINT_TO_FP and dst type
for ISD::FP_TO_SINT? Why not unify to dst type.
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:1996
setOperationAction(ISD::SCALAR_TO_VECTOR, MVT::v16f16, Custom);
+ setOperationAction(ISD::SINT_TO_FP, MVT::v16i16, Legal);
+ setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::v16i16, Legal);
----------------
How do we know it covert to v16f16? Is it possible convert to v16f32?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:2054
+ // vcvttph2[u]dq v4f16 -> v4i32/64, v2f16 -> v2i32/64
+ setOperationAction(ISD::FP_TO_SINT, MVT::v2f16, Custom);
+ setOperationAction(ISD::STRICT_FP_TO_SINT, MVT::v2f16, Custom);
----------------
Why it is not v2i16?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19998
+ SDLoc dl(Op);
+ SDValue InVec = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, MVT::v2i64, Src);
+ if (IsStrict) {
----------------
Should this node be chained to Op.getOperand(0) for strict FP and convert
operation be chained to this node?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22003
+ MakeLibCallOptions CallOptions;
+ return makeLibCall(DAG, LC, VT, In, CallOptions, SDLoc(Op)).first;
+ }
----------------
InChain for strict FP?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22014
+ SDValue Res = DAG.getNode(ISD::CONCAT_VECTORS, DL, MVT::v8f16, In,
+ DAG.getUNDEF(MVT::v4f16));
+ if (IsStrict)
----------------
Is there any case for v3f16?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31087
+ // Now widen to 128 bits.
+ unsigned NumConcats = 128 / TmpVT.getSizeInBits();
+ MVT ConcatVT = MVT::getVectorVT(EleVT.getSimpleVT(), 8 * NumConcats);
----------------
Is it possible the type is i3/i5/i7?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31265
+ if (Src.getValueType().getVectorElementType() == MVT::i16)
+ return;
+
----------------
Where is vXi16 handle? Is it promoted to vXi32 finally?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31280
+ unsigned Opc = IsSigned ? X86ISD::CVTSI2P : X86ISD::CVTUI2P;
+ Results.push_back(DAG.getNode(Opc, dl, MVT::v8f16, Src));
+ }
----------------
Isn't the result type changed to v8f16? Why we don't extract sub-vector here?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:49327
+ // UINT_TO_FP(vXi33~63) -> UINT_TO_FP(ZEXT(vXi33~63 to vXi64))
+ if (InVT.isVector() && VT.getVectorElementType() == MVT::f16) {
+ unsigned ScalarSize = InVT.getScalarSizeInBits();
----------------
Need to check Subtarget.hasFP16() ?
================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:8194
+ }
+ def : InstAlias<OpcodeStr#"x\t{$src, $dst|$dst, $src}",
+ (!cast<Instruction>(NAME # "Z128rr") VR128X:$dst,
----------------
What is the alias used for? Can't it be distinguished from operand?
================
Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:8193
+ defm Z128 : avx512_vcvt_fp<opc, OpcodeStr, v8f16x_info, v2f64x_info,
null_frag,
+ null_frag, sched.XMM, "{1to2}", "{x}", f128mem,
+ VK2WM>, EVEX_V128;
----------------
Why null_frag instead of X86vfpround?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105265/new/
https://reviews.llvm.org/D105265
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
