[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:
> pengfei wrote:
> > 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.
> I completely understand where you're coming from. What we do lose is the ability for code editors to display the doxygen when using the intrinsic (or mouseover the code). Are there any particular intrinsics that we could do with having comments closer at hand - ones that take rounding modes that its tricky to remember the enum/defines for or implicit load/store alignments come to mind?
> 
> I'm not sure about the idea of linking to external docs for specs - do we have a style guide policy on this?
> Are there any particular intrinsics that we could do with having comments closer at hand
I only found 3 ones from avx512fintrin.h, anyway, I copied here.

> ones that take rounding modes that its tricky to remember the enum/defines for or implicit load/store alignments come to mind
Unfortunately, we didn't add doc for them when enabling avx512 intrinsics.

> I'm not sure about the idea of linking to external docs for specs - do we have a style guide policy on this?
I was thinking some thing like "See https://llvm.org/LICENSE.txt for license information." in most source files. But I agree doxygen helps for code editors. I didn't think of them simply because I never used them :)
I had some thought about writing a tool to help transporting intrinsic guide info to doxygen, but haven't yet found time to do it.

Anyway, I guess this is not the block issue for this series patches, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263