[PATCH] D78190: Add Bfloat IR type

Mon Apr 27 10:43:38 PDT 2020

stuij marked an inline comment as done.
stuij added inline comments.

================
Comment at: llvm/docs/LangRef.rst:2896
+   * - ``bfloat``
+     - 16-bit brain floating-point value (8-bit mantissa)
+
----------------
scanon wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > scanon wrote:
> > > > bfloat and fp128 should agree w.r.t. whether or not the implicit bit counts. Either 7 and 112 or 8 and 113. Also, we should use "significand" instead of "mantissa". "Mantissa" has slipped in in a bunch of places, but "significand" is the IEEE 754 terminology, and we should follow it.
> > > I agree with Steve.  In general, there's no reason for these descriptions to be as terse as they are, especially for the non-standard formats.  Someone reading IR and seeing `bfloat` for the first time is going to come here and not be any wiser unless they figure out the right web search.
> > Hmm, now this reads more like a rationale than documentation.  I would suggest:
> > 
> > > 16-bit "brain" floating-point value (7-bit significand).  Provides the same number of exponent bits as ``float``, so that it matches its dynamic range, just with greatly reduced precision.  Used in Intel's AVX-512 BF16 extensions and ARM's ARMv8.6-A extensions, among others.
> Yup, I agree. The important thing here is that someone can figure out what it is (the top half of a float); it's ok for them to have to do some reading to figure out *why* it is.
Does look cleaner. Done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78190/new/

https://reviews.llvm.org/D78190