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

Thu May 21 09:41:48 PDT 2020

fpetrogalli added inline comments.

================
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2198

+static void emitNeonTypeDefs(const std::string& types, raw_ostream &OS) {
+  std::string TypedefTypes(types);
----------------
stuij wrote:
> fpetrogalli wrote:
> > 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.
> Yes, related to bfloat. We're emitting that code twice now.
> 
> I can make a new patch, but I'm not sure if the effort justifies the in my mind small amount of gain in clarity. It's basically just pasting the removed part on the left into this function. If you disagree, tell me, and I will create the extra patch.
Thank you. I didn't notice you were invoking this twice. Make sense to have it in a separate function, in this patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708