On Wed, Oct 14, 2020 at 5:06 PM Hongyu Wang <wwwhhhyyy...@gmail.com> wrote:
>
> Hi Uros,
>
> Sorry for my misunderstanding. The test is for the correctness check
> of intrinsic header.
> I have add -muintr to x86gprintrin-{1,2,3,4,5}.c.
>
> UINTR is 64bit only, so I add them with dg-additional-option.
>
> Updated patch. If you agree, we will check-in the attached patch.
Yes, the patch is OK.
Thanks,
Uros.
> Thanks for your help.
>
> H.J. Lu <hjl.to...@gmail.com> 于2020年10月14日周三 下午9:35写道：
> >
> > 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.
