[PATCH] D95447: [RISCV] Add support for RVV int<->fp & fp<->fp conversions

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:817
+    SDValue IntermediateRound =
+        DAG.getFPExtendOrRound(Op.getOperand(0), DL, InterVT);
+    return DAG.getFPExtendOrRound(IntermediateRound, DL, VT);
----------------
I'm a bit surprised this works. Back to back FP_EXTEND seems like an obvious DAG combine.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:822
+    // RVV can only do fp_round to types half the size as the source. We
+    // custom-lower f64->f16 rounds via RVV's round-towards-odd float
+    // conversion instruction.
----------------
I think it's in the spec as round-to-odd


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:850
+    MVT SrcEltVT = Src.getSimpleValueType().getVectorElementType();
+    uint64_t EltSize = EltVT.getSizeInBits();
+    uint64_t SrcEltSize = SrcEltVT.getSizeInBits();
----------------
getSizeInBits returns an unsigned.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:852
+    uint64_t SrcEltSize = SrcEltVT.getSizeInBits();
+    assert(isPowerOf2_64(EltSize) && isPowerOf2_64(SrcEltSize) &&
+           "Unexpected vector element types");
----------------
Once the uint64_ts above are unsigned this can be isPowerOf2_32.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:880
+        // One narrowing int_to_fp, then an fp_round.
+        // TODO: Is this double-rounding correct?
+        assert(EltVT == MVT::f16 && "Unexpected [US]_TO_FP lowering");
----------------
Anything large enough to trigger rounding on the float conversion, is too large to fit in f16 anyway isn't it? So it will become infinity I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95447