Support "Zfh" extension according to https://github.com/riscv/riscv-isa-manual/blob/zfh/src/zfh.tex.
Presumably we don't need the test for hasStdExtZfh, since hasStdExtF would match anyway.
If we have both RV32IF and RV32IFD, I wonder if we should also have RV32IFZfh, instead of only RV32IFDZfh.
This test should probably use a hard-float ABI, to cut down on the fmvs.
Historically for the RISC-V backend we've split patches into the MC layer changes and the codegen changes in order to make them easier to review. Do you think you'd be able to do the same here?
Zfh extension is similar to F and D extension. I use F and D extension as the reference to implement Zfh. So, I implement the feature in one patch. I have no separate patches for MC and codegen on my hands.
This needs to be rebased. This file uses MCRegister instead of Register now.
This needs to be rebased to use NODE_NAME_CASE macros.
"halfing point" here looks like a bad search and replace of "float". Can you change to "half precision floating point" and check if this occurs anywhere else.
Are we missing coverage of conversions to/from double with D extension enabled?
I had added an inline comment for half-arith.ll nitpicking that it should probably use a hard-float ABI, to cut down on the fmvs. Now it has test checks for both soft float and hard float ABIs. @HsiangKai Is this because you think there is value in testing both scenarios in that file (and several others)?
My intuition is that there should be tests for the various instructions that are only hard-float ABIs, and tests specifically for all the different calling conventions that use a very limited set of instructions.
Nit: I'd put FPR16 before FPR32 in this file (here and below).
Comment needs updating.
Hm, this code is a bit silly, we only need to look at one of the arrays as the registers all alias each other.
Since this code is not already ISA/ABI-dependent this should be fine?
Needs updating to mention RISCVReg16? Though this is currently backwards; the 64-bit register is the canonical one given the diff in RISCVAsmParser...
Overall this still looks good to me. Could you please just fix the following inconsistency?
; RUN: llc -mtriple=riscv32 -mattr=+experimental-zfh -verify-machineinstrs \ ; RUN: -target-abi ilp32f < %s | FileCheck -check-prefix=RV32IZFH-ILP32F %s ; RUN: llc -mtriple=riscv32 -mattr=+experimental-zfh -verify-machineinstrs \ ; RUN: -target-abi ilp32f < %s | FileCheck -check-prefix=RV32IF-ILP32F %s
Besides being inconsistent, the latter just seems wrong to me.
If you're going to make changes to this patch. Can you move this into a final else and pull out the "break;" Similar to the code in convertValVTToLocVT. Sorry I missed it earlier.
I thought I'd gone through it thoroughly last time but apparently not :( oh well hopefully the last set of nits.
Please also sort this one.
Needs sorting too.
Comment should be updated to reflect the sorted order. It's also unclear which combinations are invalid; currently it could be interpreted as f16 only being an issue on RV32.
Still not clear that:
Maybe it's best to generalise the opening paragraph to something like: