[X86] VP2INTERSECT clangClosedPublicActions

Authored by xiangzhangllvm on May 23 2019, 11:55 PM.
Tags
• Restricted Project
• Restricted Project
Subscribers

Details

Reviewers
 craig.topper LuoYuanke annita.zhang pengfei spatel RKSimon smaslov
Commits
rL362196: [X86] Add VP2INTERSECT instructions
rC362196: [X86] Add VP2INTERSECT instructions
rGcc3629d545a8: [X86] Add VP2INTERSECT instructions
Summary

Support intel AVX512 VP2INTERSECT instructions in clang

Diff Detail

Repository
rC Clang

Event Timeline

Herald added a project: Restricted Project. May 23 2019, 11:55 PM
Herald added subscribers: cfe-commits, mgorny.
craig.topper retitled this revision from VP2INTERSECT clang to [X86] VP2INTERSECT clang.May 24 2019, 12:11 AM
craig.topper added reviewers: .
lib/Basic/Targets/X86.cpp
520–521

This patch needs to be rebased. I modified the formatting here earlier to fix the weird indentation.

craig.topper added inline comments.May 27 2019, 10:11 PM
39

Can you add doxygen comments for the new intrinsics? @RKSimon has been asking for it on other reviews. I forgot to say something in our internal review.

xiangzhangllvm marked an inline comment as done.May 27 2019, 10:39 PM
39

OK! But I really find many ntrinsicxxx.h have no doxygen comments, Is it like this format: ?

```/// Rounds up each element of the 128-bit vector of [4 x float] to an
///    integer and returns the rounded values in a 128-bit vector of
///    [4 x float].
///
///
/// \code
/// __m128 _mm_ceil_ps(__m128 X);
/// \endcode
///
/// This intrinsic corresponds to the <c> VROUNDPS / ROUNDPS </c> instruction.
///
/// \param X
///    A 128-bit vector of [4 x float] values to be rounded up.
/// \returns A 128-bit vector of [4 x float] containing the rounded values.
#define _mm_ceil_ps(X)       _mm_round_ps((X), _MM_FROUND_CEIL)```
craig.topper added inline comments.May 27 2019, 10:43 PM
39

Yeah that looks right. Someone was kind enough to provide comments for some of the header files a few years ago, but it wasn't complete. I know avx512* especially are missing comments.

xiangzhangllvm marked 2 inline comments as done.May 28 2019, 12:01 AM
RKSimon added inline comments.May 28 2019, 3:45 AM
39

The hope is that these will get added as time goes on - usually it just requires a copy + paste of existing comments and tweak for ymm/zmm/whatever.

New intrinsics should always include documentation since the person adding it is the most likely to be able to correct describe it.

xiangzhangllvm marked an inline comment as done.May 28 2019, 4:41 AM
39

Yes, That make sense!

Hi Dear friends, could you help merge this patch? Thank you very much!

craig.topper added inline comments.May 30 2019, 7:09 PM
40

This doesn't really say anything about what's in the mask registers. I don' think any of the MODRM encoding details are relevant here. I think a better description would be what's in the table in the ISE manual. "Store, in an even/odd pair of mask registers,
the indicators of the locations of value
matches between dwords in
xmm3/m128/m32bcst and xmm2."

47

Drop the "AVX512-". Add the proper D/Q suffix. This should be the instruction mnemonic

54

ponit?

xiangzhangllvm marked 3 inline comments as done.

Done, Thank you very much!

This revision is now accepted and ready to land.May 30 2019, 8:36 PM
Closed by commit rL362196: [X86] Add VP2INTERSECT instructions (authored by pengfei, committed by ). May 30 2019, 11:07 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. May 30 2019, 11:07 PM
Herald added a subscriber: llvm-commits.