[PATCH] D153848: [RISCV] Apply promotion for f16 vector ops when only have zvfhmin.

[Michael Maitland via Phabricator via llvm-commits]

michaelmaitland added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5508
   case ISD::SINT_TO_FP:
   case ISD::UINT_TO_FP:
+    if (Op.getValueType().isVector() &&
----------------
Why don't we reuse the logic below? 

```
// Widening
if (IsInt2FP) {
        // Do a regular integer sign/zero extension then convert to float.
// Narrowing 
if (IsInt2FP) {
        // One narrowing int_to_fp, then an fp_round.
```

`narrowing int_to_fp then an fp_round` seems to be what we're doing here right?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:6036
   case ISD::VP_SINT_TO_FP:
   case ISD::VP_UINT_TO_FP:
+    if (Op.getValueType().isVector() &&
----------------
Similar question, is there a good reason to not have this in `lowerVPFPIntConvOp`?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:394
     setOperationAction(ISD::FREM, MVT::bf16, Promote);
     // FIXME: Need to promote bf16 FCOPYSIGN to f32, but the
     // DAGCombiner::visitFP_ROUND probably needs improvements first.
----------------
Does this need to be fixed as a part of this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153848