On Wed, Oct 14, 2020 at 6:31 AM Hongyu Wang via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Uros Bizjak <ubiz...@gmail.com> 于2020年10月14日周三 下午7:19写道: > > > > > > Please also add -muintr to g++.dg/other/i386-{2,3}.C and > > > > >> > > gcc.target/i386-sse-{12,13,14,22,23}.c. This will test new > intrinsics > > > > >> > > header. > > > > >> > > > > > > >> > > > > > >> > Thanks for your review. We found that without adding -muintr, > the intrinsics header could also be tested. Make-check for these file all > get passed. > > > > >> > > > > > >> > And there is no intrinsic/builtin with const int parameter. So > we remove -muintr from these files. > > > > >> > > > > >> Can your double check that relevant instructions are indeed > generated? > > > > >> Without -muintr, relevant patterns in i386.md are effectively > blocked, > > > > >> and perhaps a call to __builtin_ia32_* is generated instead. > > > > > > > > > > > > > > > Yes, in sse-14.s we have > > > > > > > > > > _clui: > > > > > .LFB136: > > > > > .cfi_startproc > > > > > pushq %rbp > > > > > .cfi_def_cfa_offset 16 > > > > > .cfi_offset 6, -16 > > > > > movq %rsp, %rbp > > > > > .cfi_def_cfa_register 6 > > > > > clui > > > > > nop > > > > > popq %rbp > > > > > .cfi_def_cfa 7, 8 > > > > > ret > > > > > .cfi_endproc > > > > > > > > Strange, without -muintr, it should not be generated, and some error > > > > about failed inlining due to target specific option mismatch shoul be > > > > emitted. > > > > > > > > Can you please investigate this a bit more? > > > > > > > > > > Because of function target attribute? > > > > I don't think so. Please consider this similar testcase: > > > > --cut here-- > > #ifndef __SSE2__ > > #pragma GCC push_options > > #pragma GCC target("sse2") > > #define __DISABLE_SSE2__ > > #endif /* __SSE2__ */ > > > > typedef double __v2df __attribute__ ((__vector_size__ (16))); > > typedef double __m128d __attribute__ ((__vector_size__ (16), > __may_alias__)); > > > > extern __inline __m128d __attribute__((__gnu_inline__, > > __always_inline__, __artificial__)) > > _mm_add_sd (__m128d __A, __m128d __B) > > { > > return (__m128d)__builtin_ia32_addsd ((__v2df)__A, (__v2df)__B); > > } > > > > #ifdef __DISABLE_SSE2__ > > #undef __DISABLE_SSE2__ > > #pragma GCC pop_options > > #endif /* __DISABLE_SSE2__ */ > > > > > > __v2df foo (__v2df a, __v2df b) > > { > > return _mm_add_sd (a, b); > > } > > --cut here-- > > > > $ gcc -O2 -mno-sse2 -S -dp sse2.c > > sse2.c: In function ‘foo’: > > sse2.c:11:1: error: inlining failed in call to ‘always_inline’ > > ‘_mm_add_sd’: target specific option mismatch > > 11 | _mm_add_sd (__m128d __A, __m128d __B) > > | ^~~~~~~~~~ > > sse2.c:24:10: note: called from here > > 24 | return _mm_add_sd (a, b); > > | ^~~~~~~~~~~~~~~~~ > > > > I'd expect some similar warning from missing -mumip. > > > > For this case, I can confirm uintr could generate similar warning without > -muintr. But > sse-{12,13,14,22,23}.c will not test intrinsic call for uintr, since it > doesn't have const > int parameter intrinsics. > > sse-{13,14,22,23}.c has > > #define extern > #define __inline > > So intrinsic will be treated as common call to builtin, then > > #pragma GCC push_options > #pragma GCC target("uintr") > > ensures the builtin could be expanded correctly. > > I think the intrinsic call test should be in uintr-1.c, so it is redundant > to add -muintr in sse-{12,13,14,22,23}.c > or x86gprintrin-*.c.
Please add UINTR intrinsic tests to x86gprintrin-*.c to cover such usages. > > > > Uros. -- H.J.