[PATCH] D81304: [llvm][SveEmitter] Emit the bfloat version of `svld1ro`.

Sander de Smalen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 8 02:41:00 PDT 2020


sdesmalen added a comment.

Thanks @fpetrogalli, the changes look good to me. Just added some minor comments.



================
Comment at: clang/include/clang/Basic/AArch64SVEACLETypes.def:63
 
-SVE_VECTOR_TYPE("__SVFloat16_t", SveFloat16, SveFloat16Ty, 8, 16, true, true)
-SVE_VECTOR_TYPE("__SVFloat32_t", SveFloat32, SveFloat32Ty, 4, 32, true, true)
-SVE_VECTOR_TYPE("__SVFloat64_t", SveFloat64, SveFloat64Ty, 2, 64, true, true)
+SVE_VECTOR_TYPE("__SVBFloat16_t", SveBFloat16, SveBFloat16Ty, 8, 16, false, false, true)
+SVE_VECTOR_TYPE("__SVFloat16_t", SveFloat16, SveFloat16Ty, 8, 16, true, true, false)
----------------
nit: I'd move this below __SVFloat64_t separated by a newline.


================
Comment at: clang/utils/TableGen/SveEmitter.cpp:366
     S += "v";
-  else if (!Float)
+  else if (!isFloat() && !isBFloat())
     switch (ElementBitwidth) {
----------------
Maybe it's worth adding a
  bool isFloatingPoint() const { return Float || BFloat; }
to avoid having to write both `isFloat` and `isBFloat` everywhere.


================
Comment at: clang/utils/TableGen/SveEmitter.cpp:385
+    switch (ElementBitwidth) {
+    case 16: S += "y"; break;
+    default: llvm_unreachable("Unhandled case!");
----------------
It's easier to just assert that ElementBitwidth == 16.


================
Comment at: clang/utils/TableGen/SveEmitter.cpp:942
+  if (T.isBFloat()) {
+    switch (T.getElementSizeInBits()) {
+    case 16:
----------------
It's easier to just assert that ElementBitwidth == 16


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81304





More information about the cfe-commits mailing list