[PATCH] D105263: [X86] AVX512FP16 instructions enabling 1/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/lib/Headers/avx512fp16intrin.h:38
+
+static __inline__ _Float16 __DEFAULT_FN_ATTRS512 _mm512_cvtsh_h(__m512h __a) {
+  return __a[0];
----------------
RKSimon wrote:
> I realize its a lot of work, but is there any chance that we could get doxygen comments to document these intrinsics?
I'm hesitating not only for the work but also the effect. We have about 1K new intrinsics and more than 5K LOC in total in the two header files. Adding the doxygen comments will make the readability worse and increase the difficulty in review. It's also a burden in maintaining the correctness.
Do you think it's feasible to only add a link to intrinsic guide? We have decided to only using link that points intrinsic guide in our product compiler. Using one source is friendly to maintainess. And I think intrinsic guide is also easy to use that doxygen.


================
Comment at: llvm/lib/Target/X86/X86Subtarget.h:748
   bool hasVLX() const { return HasVLX; }
+  bool hasFP16() const { return HasFP16; }
   bool hasPKU() const { return HasPKU; }
----------------
RKSimon wrote:
> I'm a little worried this might get confused with hasF16C - am I just being over cautious?
Make sense. How about `hasAVX512FP16`? I can update the name as a followup patch once these patches merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263