[PATCH] D139398: [AMDGPU] Add bf16 storage support

[Matt Arsenault via Phabricator via cfe-commits]

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