[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi @stuij ,

thank you for working on this.

Would it make sense to add a test that includes the header file you have created?

Regards,

Francesco



================
Comment at: clang/include/clang/Basic/arm_neon_incl.td:293
+
+  string CartesianProductWith = "";
 }
----------------
What is this for?


================
Comment at: clang/utils/TableGen/NeonEmitter.cpp:628
     S += "x" + utostr(getNumElements());
+
   if (NumVectors > 1)
----------------
Remove me.


================
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2198
 
+static void emitNeonTypeDefs(const std::string& types, raw_ostream &OS) {
+  std::string TypedefTypes(types);
----------------
Is this related to the changes for bfloat? Or is it a just a refactoring that it is nice to have? If the latter, please consider submitting it as a separate patch. If both refactoring and BF16 related, at the moment it is not possible to see clearly which changes are BF16 specific, so please do submit the refactoring first.


================
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2617
+
+  OS << "#endif";
 }
----------------
Missing `\n`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708