[PATCH] D70089: [VFABI] Add LLVM internal mangling for vector functions.

Tue Nov 12 06:53:38 PST 2019

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

================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:50
   AVX512,       // x86 AVX512
-  Unknown       // Unknown ISA
+  LLVM_VF,      // LLVM internal ISA for functions that are not
+  // attached to an existing ABI via name mangling.
----------------
fpetrogalli wrote:
> sdesmalen wrote:
> > nit: maybe drop the `_VF` and use `LLVM` directly? (also for other uses in this file)
> I'd prefer `llvm::LLVM_VF`. The alternative `llvm::LLVM` is less appealing to me, because it doesn't say anything about it's meaning. In the former, at least `VF` stands for Vector Function.
> 
> Once could argue that we could move the variable into the `VFABI` namespace, so that `llvm::VFABI::LLVM` would be slightly more specific, but I'd rather not do this too because I suspect that the `llvm::LLVM_VF` ISA coudl be used also outside of a VFABI context (like we do it now for the other vector extensions).
> 
> So, overall, I think we should stay with `llvm::LLVM_VF`.
*Never* reply to a code review before having coffee. 

`llvm::VFISAKind::LLVM`will be.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70089