craig.topper added inline comments.
================ Comment at: clang/lib/Headers/avx512fp16intrin.h:254 +/// Constructs a 512-bit floating-point vector of [32 x half] from a +/// 128-bit floating-point vector of [16 x half]. The lower 256 bits +/// contain the value of the source vector. The upper 256 bits are set ---------------- 256-bit ================ Comment at: clang/lib/Headers/avx512vlfp16intrin.h:74 +static __inline__ __m256h __DEFAULT_FN_ATTRS256 _mm256_abs_ph(__m256h __A) { + return (__m256h)_mm256_and_epi32(_mm256_set1_epi32(0x7FFF7FFF), (__m256i)__A); +} ---------------- Why do we use _mm256_set1_epi32 instead of _mm256_set1_epi16? ================ Comment at: clang/lib/Headers/avx512vlfp16intrin.h:78 +static __inline__ __m128h __DEFAULT_FN_ATTRS128 _mm_abs_ph(__m128h __A) { + return (__m128h)_mm_and_epi32(_mm_set1_epi32(0x7FFF7FFF), (__m128i)__A); +} ---------------- Same question ================ Comment at: llvm/include/llvm/IR/RuntimeLibcalls.def:290 HANDLE_LIBCALL(FPEXT_F16_F128, "__extendhftf2") +HANDLE_LIBCALL(FPEXT_F16_F80, "__extendhfxf2") HANDLE_LIBCALL(FPEXT_F32_F64, "__extendsfdf2") ---------------- Is this tested in this patch? ================ Comment at: llvm/lib/Target/X86/X86FastISel.cpp:58 bool X86ScalarSSEf32; + bool X86ScalarAVXf16; ---------------- AVX here should maybe be AVX512, but maybe this is pointing out that this name is bad. Would X86ScalarXMMf* be better? ================ Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:13537 + unsigned MovOpc = 0; + if (EltVT == MVT::f16) { + MovOpc = X86ISD::MOVSH; ---------------- Drop curly braces on these. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105263/new/ https://reviews.llvm.org/D105263 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits