[PATCH] D89105: [X86] Support Intel avxvnni

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Headers/avxvnniintrin.h:31
+
+#include "commonvnniintrin.h"
+
----------------
pengfei wrote:
> RKSimon wrote:
> > LiuChen3 wrote:
> > > LiuChen3 wrote:
> > > > RKSimon wrote:
> > > > > Is having a commonvnniintrin.h header the approach gcc/icc are taking?
> > > > GCC doesn't have commonvnniintrin.h headr, they only have avxvnniintrin.h. For ICC, I will check with this and give you answer later.
> > > GCC and ICC doesn't break intrinsics into multiple headers.
> > Is there anyway that we can avoid this as well then? I get worried when header layouts start to diverge...
> Hi Simon, why we need to keep the same layout with GCC and ICC?
> We always create new header files for new instructions. We don't allow user include these headers directly. So I think it doesn't matter if GCC and ICC create the same headers or not.
As far as I know our header layout is largely the same as gcc for existing intrinsics. Can we just keep them the same?


================
Comment at: clang/lib/Headers/commonvnniintrin.h:48
+/// \endoperation
+#define _mm256_dpbusd_epi32(__S, __A, __B) \
+  (__m256i)__builtin_ia32_vpdpbusd256((__v8si)__S, (__v8si)__A, (__v8si)__B)
----------------
Arguments to macros shouldn't start with __


================
Comment at: clang/lib/Headers/commonvnniintrin.h:49
+#define _mm256_dpbusd_epi32(__S, __A, __B) \
+  (__m256i)__builtin_ia32_vpdpbusd256((__v8si)__S, (__v8si)__A, (__v8si)__B)
+
----------------
Because this is a macro this needs to have parentheses around S, A, and B.


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

https://reviews.llvm.org/D89105