Recently, bf16 type has been added in RISCV psABI. Now we need to support this datatype in LLVM backend. Right now, according to psABI, we basically address __bf16 like FP16.
Details
Diff Detail
Event Timeline
Sorry to not have specifics on this - I'm working on a patch in this same area and as you've found as well, it's quite fiddly. Using the __truncsfhf2 and __extendhfsf2 libcalls isn't correct - they expect 16-bit floats in a different format to bfloat16.
I have checked compiler-rt/lib/builtins. There is truncsfbf2 for single-bfloat conversion, but I cannot find any library for bfloat extend.
libgcc has defined truncsfbf2, truncdfbf2, truncxfbf2, trunctfbf2, trunchfbf2, truncbfhf2 and extendbfsf2
I guess this might made me seems like a GNUism, but following the same rule should be easier for our life :P
Ref: https://github.com/gcc-mirror/gcc/commit/c2565a31c1622ab0926aeef4a6579413e121b9f9
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2458 | If the libcall doesn't exist, you can cast it to i16, any_extend to i32, and shift it left by 16 and bitcast it to f32. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2458 | Which is what the default lowering in LegalizeDAG.cpp does. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2438 | This is an incorrect conversion. We can't truncate the mantissa. It would turn some nan encodings into infinity. Probably other issues I haven't thought of yet. | |
2438 | We need to use truncsfbf2 | |
2469 | No need for else after an if that returns | |
2486 | Doesn't this need to be i64 on 64 bit target? | |
2487 | This creates an MVT::i16 after type legalization which is illegal. | |
2493 | You share the shift code between both paths by using XLenVT. | |
2495 | Use getShiftAmountConstant | |
2509 | No need for else after if returns |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2458 | We cannot directly use the default lowering in LegalizeDAG.cpp. It uses a bitcast from i32 to f32. However, i32 is not a legal type in RV64 backend. | |
2487 | In fact, without Zfbfmin enabled, bf16 type isn't legal and this condition will never happen. If Zfbfmin is enabled, maybe we can use FCVT.S.BF16 to directly convert BF16 value to an FP32 value. |
The first version of this patch came in May 24th, earlier than a similar patch implemented by @asb https://reviews.llvm.org/D151663. Why could @asb's patch be accepted first?
I think in this case, it should be to provide review suggestions for modification, instead of immediately accepting other people's improvement patch, which has a great impact on the enthusiasm of contributors :)
llvm/test/CodeGen/RISCV/bfloat.ll | ||
---|---|---|
1 | This file already exists in trunk. Not sure if is the same or not? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
428 | Maybe I can remove this part. In my first version of this patch, the expand is needed for LoadExt and TruncStore. | |
432 | Since we use also libcall in the lowerFP_TO_BF16 function, I don't think we need to use "Libcall" here for SoftABI. | |
2461 | Although bf16 type isn't legal, that does not mean this condition will never happen. |
This wasn't needed by D151663? Is there a test case missing from D151663 that requires this?