[PATCH] D151313: [RISCV][BF16] Make backend type bf16 to follow the psABI
Jun Sha via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 28 01:05:52 PDT 2023
joshua-arch1 abandoned this revision.
joshua-arch1 added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:428
setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::f16, Expand);
+ setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::bf16, Expand);
setTruncStoreAction(MVT::f32, MVT::f16, Expand);
----------------
craig.topper wrote:
> This wasn't needed by D151663? Is there a test case missing from D151663 that requires this?
Maybe I can remove this part. In my first version of this patch, the expand is needed for LoadExt and TruncStore.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:432
+ setOperationAction(ISD::FP_TO_BF16, MVT::f32, Custom);
+ setOperationAction(ISD::BF16_TO_FP, MVT::f32, Custom);
setOperationAction(ISD::IS_FPCLASS, MVT::f32, Custom);
----------------
craig.topper wrote:
> D151663 uses
>
> ```
> setOperationAction(ISD::FP_TO_BF16, MVT::f32,
> Subtarget.isSoftFPABI() ? LibCall : Custom);
> ```
>
> Is that difference important?
Since we use also libcall in the lowerFP_TO_BF16 function, I don't think we need to use "Libcall" here for SoftABI.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2461
+ if (Op.getOperand(0).getValueType() == MVT::bf16) {
+ Op0 =
+ DAG.getNode(ISD::ANY_EXTEND, DL, XLenVT,
----------------
craig.topper wrote:
> Why wasn't this part needed in D151663?
Although bf16 type isn't legal, that does not mean this condition will never happen.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151313/new/
https://reviews.llvm.org/D151313
More information about the llvm-commits
mailing list