[PATCH] D78190: Add Bfloat IR type

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 23 06:59:38 PDT 2020


SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

This direction of an IR type was taken after a discussion on the list. All previous comments have been addressed, and the changes here all look very reasonable to me. So, LGTM, but I think we need a LGTM from someone else too. So please wait for one more LGTM.



================
Comment at: llvm/include/llvm/IR/DataLayout.h:655
     return TypeSize::Fixed(16);
+  case Type::BfloatTyID:
+    return TypeSize::Fixed(16);
----------------
Nit:

  case Type::HalfTyID:
  case Type::BfloatTyID:
    return TypeSize::Fixed(16);


================
Comment at: llvm/lib/Support/APFloat.cpp:3266
 
+APInt IEEEFloat::convertBfloatAPFloatToAPInt() const {
+  assert(semantics == (const llvm::fltSemantics *)&semBfloat);
----------------
I am not real big fan of all the code duplication here in this file (just the constants are different). But there's enough prior art here, so good for now I think, and a nice opportunity for a refactoring follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78190





More information about the llvm-commits mailing list