[PATCH] D139398: [AMDGPU] Add bf16 storage support
Matt Arsenault via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 9 09:39:57 PST 2022
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5573-5576
+ SDLoc SL(Op);
+ return DAG.getNode(
+ ISD::FP_EXTEND, SL, MVT::f32,
+ DAG.getNode(ISD::BITCAST, SL, MVT::f16, Op->getOperand(0)));
----------------
arsenm wrote:
> Pierre-vh wrote:
> > arsenm wrote:
> > > ExpandNode covers lowering BF16_TO_FP. It also has a shift by 16-bits into the high bits. Is this correct?
> > Ah I didn't know that, though as long as we use custom lowering, and our FP_TO_BF16/BF16_TO_FP methods are consistent, it should be fine, no?
> bfloat16 has the same number of exponent bits in the same high bits as f32; I kind of think the idea is you can just do a bitshift and then operate on f32? I think the fp_extend here is wrong
The default legalization also looks wrong to me. I don't understand why it isn't shifting down the mantissa bit
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139398/new/
https://reviews.llvm.org/D139398
More information about the cfe-commits
mailing list