[PATCH] D41507: avxintrin.h documentation fixes and updates
Katya Romanova via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 21 17:10:52 PST 2017
kromanova added inline comments.
================
Comment at: lib/Headers/avxintrin.h:1668
/// operation to use: \n
-/// 0x00 : Equal (ordered, non-signaling)
-/// 0x01 : Less-than (ordered, signaling)
-/// 0x02 : Less-than-or-equal (ordered, signaling)
-/// 0x03 : Unordered (non-signaling)
-/// 0x04 : Not-equal (unordered, non-signaling)
-/// 0x05 : Not-less-than (unordered, signaling)
-/// 0x06 : Not-less-than-or-equal (unordered, signaling)
-/// 0x07 : Ordered (non-signaling)
-/// 0x08 : Equal (unordered, non-signaling)
-/// 0x09 : Not-greater-than-or-equal (unordered, signaling)
-/// 0x0a : Not-greater-than (unordered, signaling)
-/// 0x0b : False (ordered, non-signaling)
-/// 0x0c : Not-equal (ordered, non-signaling)
-/// 0x0d : Greater-than-or-equal (ordered, signaling)
-/// 0x0e : Greater-than (ordered, signaling)
-/// 0x0f : True (unordered, non-signaling)
-/// 0x10 : Equal (ordered, signaling)
-/// 0x11 : Less-than (ordered, non-signaling)
-/// 0x12 : Less-than-or-equal (ordered, non-signaling)
-/// 0x13 : Unordered (signaling)
-/// 0x14 : Not-equal (unordered, signaling)
-/// 0x15 : Not-less-than (unordered, non-signaling)
-/// 0x16 : Not-less-than-or-equal (unordered, non-signaling)
-/// 0x17 : Ordered (signaling)
-/// 0x18 : Equal (unordered, signaling)
-/// 0x19 : Not-greater-than-or-equal (unordered, non-signaling)
-/// 0x1a : Not-greater-than (unordered, non-signaling)
-/// 0x1b : False (ordered, signaling)
-/// 0x1c : Not-equal (ordered, signaling)
-/// 0x1d : Greater-than-or-equal (ordered, non-signaling)
-/// 0x1e : Greater-than (ordered, non-signaling)
-/// 0x1f : True (unordered, signaling)
+/// 00h: Equal (ordered, non-signaling) \n
+/// 01h: Less-than (ordered, signaling) \n
----------------
lebedev.ri wrote:
> While i'm incompetent to actually review this, i have a question.
> //Why// replace code-friendly `0x00` with `00h`?
> And, //why// is this hex in the first place? Why not decimals?
>
>> why is this hex in the first place? Why not decimals?
There are a few reasons that I could think of:
(1) for consistency with the defines in the header file describing these values:
#define _CMP_EQ_OQ 0x00 /* Equal (ordered, non-signaling) */
#define _CMP_LT_OS 0x01 /* Less-than (ordered, signaling) */
#define _CMP_LE_OS 0x02 /* Less-than-or-equal (ordered, signaling) */
#define _CMP_UNORD_Q 0x03 /* Unordered (non-signaling) */
#define _CMP_NEQ_UQ 0x04 /* Not-equal (unordered, non-signaling) */
(2) This immediate is 5 bits, so when using hex, it's much easier to tell which bits are set/not set/
(3) For consistency with Intel's and AMD's documentation for intrinsics and corresponding instructions.
I s developers prefer to have it in 0x format, this change it was done for consistency.
AMD and Intel manuals use the 'h' suffix style
================
Comment at: lib/Headers/avxintrin.h:1668
/// operation to use: \n
-/// 0x00 : Equal (ordered, non-signaling)
-/// 0x01 : Less-than (ordered, signaling)
-/// 0x02 : Less-than-or-equal (ordered, signaling)
-/// 0x03 : Unordered (non-signaling)
-/// 0x04 : Not-equal (unordered, non-signaling)
-/// 0x05 : Not-less-than (unordered, signaling)
-/// 0x06 : Not-less-than-or-equal (unordered, signaling)
-/// 0x07 : Ordered (non-signaling)
-/// 0x08 : Equal (unordered, non-signaling)
-/// 0x09 : Not-greater-than-or-equal (unordered, signaling)
-/// 0x0a : Not-greater-than (unordered, signaling)
-/// 0x0b : False (ordered, non-signaling)
-/// 0x0c : Not-equal (ordered, non-signaling)
-/// 0x0d : Greater-than-or-equal (ordered, signaling)
-/// 0x0e : Greater-than (ordered, signaling)
-/// 0x0f : True (unordered, non-signaling)
-/// 0x10 : Equal (ordered, signaling)
-/// 0x11 : Less-than (ordered, non-signaling)
-/// 0x12 : Less-than-or-equal (ordered, non-signaling)
-/// 0x13 : Unordered (signaling)
-/// 0x14 : Not-equal (unordered, signaling)
-/// 0x15 : Not-less-than (unordered, non-signaling)
-/// 0x16 : Not-less-than-or-equal (unordered, non-signaling)
-/// 0x17 : Ordered (signaling)
-/// 0x18 : Equal (unordered, signaling)
-/// 0x19 : Not-greater-than-or-equal (unordered, non-signaling)
-/// 0x1a : Not-greater-than (unordered, non-signaling)
-/// 0x1b : False (ordered, signaling)
-/// 0x1c : Not-equal (ordered, signaling)
-/// 0x1d : Greater-than-or-equal (ordered, non-signaling)
-/// 0x1e : Greater-than (ordered, non-signaling)
-/// 0x1f : True (unordered, signaling)
+/// 00h: Equal (ordered, non-signaling) \n
+/// 01h: Less-than (ordered, signaling) \n
----------------
kromanova wrote:
> lebedev.ri wrote:
> > While i'm incompetent to actually review this, i have a question.
> > //Why// replace code-friendly `0x00` with `00h`?
> > And, //why// is this hex in the first place? Why not decimals?
> >
> >> why is this hex in the first place? Why not decimals?
> There are a few reasons that I could think of:
>
> (1) for consistency with the defines in the header file describing these values:
> #define _CMP_EQ_OQ 0x00 /* Equal (ordered, non-signaling) */
> #define _CMP_LT_OS 0x01 /* Less-than (ordered, signaling) */
> #define _CMP_LE_OS 0x02 /* Less-than-or-equal (ordered, signaling) */
> #define _CMP_UNORD_Q 0x03 /* Unordered (non-signaling) */
> #define _CMP_NEQ_UQ 0x04 /* Not-equal (unordered, non-signaling) */
>
> (2) This immediate is 5 bits, so when using hex, it's much easier to tell which bits are set/not set/
> (3) For consistency with Intel's and AMD's documentation for intrinsics and corresponding instructions.
>
>
> I s developers prefer to have it in 0x format, this change it was done for consistency.
> AMD and Intel manuals use the 'h' suffix style
> Why replace code-friendly 0x00 with 00h?
For consistency with some other intrinsics doxygen documentation using 'h' notation.
I looked little further and noticed that around 60% of the doxygen comments are using '0x' notation, while around 40% are using 'h' notation. It's inconsistent. Now we could pick one or another. If someone has a strong opinion, please speak up. I personally prefer '0x' notation.
Doug, could you please change your code review to introduce '0x' notation back. I will submit a separate code review a little later to change the rest of "h" occurrences to "0x" in the rest of intrinsics documentation.
================
Comment at: lib/Headers/avxintrin.h:2378
/* Vector replicate */
-/// \brief Moves and duplicates high-order (odd-indexed) values from a 256-bit
+/// \brief Moves and duplicates odd-indexed values from a 256-bit
/// vector of [8 x float] to float values in a 256-bit vector of
----------------
Please fit tightly into 80 chars.
https://reviews.llvm.org/D41507
More information about the cfe-commits
mailing list