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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits