Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)

[kromanova]

Summary

Here is the patch with the doxygen comments for the intrinsincs in the header file popcntintrin.h.
The doxygen comments are automatically generated based on SCE internal intrinsics document using DCG tool that I wrote.

I will submit more doxygen comments for the other intrinsic header files as soon as this patch is approved.

Here is the link to the general discussion about adding comments to x86 intrinsics headers.
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032

Here are the links to the similar code reviews for the doxygen comments in some other header files.

http://reviews.llvm.org/D8762 (closed)
http://reviews.llvm.org/D15999 (closed)
http://reviews.llvm.org/D16562 (closed)
http://reviews.llvm.org/D16913 (closed)
http://reviews.llvm.org/D17021 (closed)

[probinson]

LGTM.

One question I have, which shouldn't block this (as we've done several like this already):
Is is okay to be using C++ style comments in these headers?
(Is there a C-style comment that Doxygen recognizes?)

[gbedwell]

In D17550#360177, @probinson wrote:

One question I have, which shouldn't block this (as we've done several like this already):
Is is okay to be using C++ style comments in these headers?
(Is there a C-style comment that Doxygen recognizes?)

There are a few various formats that Doxygen supports. Looking at headers from llvm-c the most common convention appears to be JavaDoc style, although there are a few examples of other supported styles floating around the codebase. E.g. from include/llvm-c/lto.h using JavaDoc style:

/**
 * Diagnostic handler type.
 * \p severity defines the severity.
 * \p diag is the actual diagnostic.
 * The diagnostic is not prefixed by any of severity keyword, e.g., 'error: '.
 * \p ctxt is used to pass the context set with the diagnostic handler.
 *
 * \since LTO_API_VERSION=7
 */

-Greg

[echristo]

Yeah, we should be doing this. Nice catch Paul and Greg.

[kromanova]

Hello,

I don’t think it will too hard to convert C++ style doxygen comments into C style doxygen comments by writing a post-processing python script. However, at first we need to decide if we really want to do that. If so, we need to settle on the exact format. After that, I need to make sure that the comments in the new format will be rendered correctly in MS Tooltips, XCode, online documentation and PS4 internal documentation. This discussion + investigation might take a few days.

Before we start discussing the exact format, I want to make sure that we really want to change to C-style doxygen comments.
Here are my not-so-strong arguments against it:

• There currently are 257 occurrences C++ style comments in 14 other header files in /llvm/tools/clang/lib/Headers directory (I’m talking about the files that I didn’t touch). C++ style comments were there for AGES and nobody complained so far. If we decide to change C++ style doxygen comments -> C-style, we also need to change all regular C++ comments to C-style in these header files.

• c99 (and later) supports C++ style comments, while I c89 doesn’t. I’m not sure if we have users that still use c89 format and x86 intrinsic headers at the same time.

• C++ style doxygen comments are more pretty and readable compared to C-style comment (though it might be my subjective opinion).

Let me know what you think.

I will try to get Dmitri Gribenko’s opinion. He did a lot of work on doxygen in LLVM. I’m curious what he thinks about Javadoc style format.

Katya.

From: Eric Christopher [mailto:[e-mail]]
Sent: Tuesday, February 23, 2016 10:51 PM
To: [e-mail]; Romanova, Katya; Gao, Yunzhong; [e-mail]; [e-mail]; Robinson, Paul
Cc: Bedwell, Greg; [e-mail]
Subject: Re: [PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)

Yeah, we should be doing this. Nice catch Paul and Greg.

[echristo]

Those are all compelling reasons for me. Let's go with whatever you and
Dmitri think would be best for now. :)

-eric

[kromanova]

Hi Dmitri,
Could you please let us know your opinion about C++ vs C-style doxygen comments. Read this thread for ‘pro’ and ‘con’ arguments about using C++ headers. Will LLVM online documentation look proper if we decide to use C-style headers? Which style do you personally prefer to see?

Note, that if there is any complaint in the future, it will take a couple of hours to write a python script to convert from C++ to C style doxygen comments.

Katya.

From: Eric Christopher [mailto:[e-mail]]
Sent: Wednesday, February 24, 2016 6:40 PM
To: Romanova, Katya; [e-mail]; Gao, Yunzhong; [e-mail]; [e-mail]; Robinson, Paul
Cc: Bedwell, Greg; [e-mail]
Subject: Re: [PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)

Those are all compelling reasons for me. Let's go with whatever you and Dmitri think would be best for now. :)

-eric

cfe/trunk/lib/Headers/popcntintrin.h

  */
 
 #ifndef _POPCNTINTRIN_H
 #define _POPCNTINTRIN_H
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("popcnt")))
 
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///    An unsigned 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///    source operand.
 static __inline__ int __DEFAULT_FN_ATTRS
 _mm_popcnt_u32(unsigned int __A)
 {
   return __builtin_popcount(__A);
 }
 
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///    A signed 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///    source operand.
 static __inline__ int __DEFAULT_FN_ATTRS
 _popcnt32(int __A)
 {
   return __builtin_popcount(__A);
 }
 
 #ifdef __x86_64__
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///    An unsigned 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///    source operand.
 static __inline__ long long __DEFAULT_FN_ATTRS
 _mm_popcnt_u64(unsigned long long __A)
 {
   return __builtin_popcountll(__A);
 }
 
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile <x86intrin.h>
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///    A signed 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///    source operand.
 static __inline__ long long __DEFAULT_FN_ATTRS
 _popcnt64(long long __A)
 {
   return __builtin_popcountll(__A);
 }
 #endif /* __x86_64__ */
 
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* _POPCNTINTRIN_H */