[PATCH] D135932: [X86] Add AVX-IFMA instructions.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/include/clang/Basic/BuiltinsX86.def:1335-1338
 TARGET_BUILTIN(__builtin_ia32_vpmadd52huq128, "V2OiV2OiV2OiV2Oi", "ncV:128:", "avx512ifma,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52huq256, "V4OiV4OiV4OiV4Oi", "ncV:256:", "avx512ifma,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52luq128, "V2OiV2OiV2OiV2Oi", "ncV:128:", "avx512ifma,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmadd52luq256, "V4OiV4OiV4OiV4Oi", "ncV:256:", "avx512ifma,avx512vl")
----------------
Can we unify the builtins like AVXVNNI? e.g., change it to `vx512ifma,avx512vl|avxifma`


================
Comment at: clang/lib/Headers/avxifmaintrin.h:36
+/// __m128i
+/// _mm_madd52hi_avx_epu64 (__m128i __X, __m128i __Y, __m128i __Z)
+/// \endcode
----------------
Should we provide unified intrinsic `_mm_madd52hi_epu64` like AVXVNNI?


================
Comment at: llvm/test/CodeGen/X86/avx-ifma-intrinsics.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+avxifma --show-mc-encoding | FileCheck %s --check-prefixes=AVX-X86
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avxifma --show-mc-encoding | FileCheck %s --check-prefixes=AVX-X64
----------------
Use `AVX,AVX-X86` etc. to reduce the duplicated check.


================
Comment at: llvm/test/CodeGen/X86/avx-ifma-intrinsics.ll:22
+; AVX512-X86:       # %bb.0:
+; AVX512-X86-NEXT:    vpmadd52huq %xmm2, %xmm1, %xmm0 # encoding: [0x62,0xf2,0xf5,0x08,0xb5,0xc2]
+; AVX512-X86-NEXT:    retl # encoding: [0xc3]
----------------
I think we should expect VEX encoding here with comment `EVEX TO VEX Compression encoding`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135932