> -----Original Message----- > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Uros Bizjak > Sent: Tuesday, September 19, 2017 6:13 PM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > Cc: gcc-patches@gcc.gnu.org > Subject: Re: 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > > On Tue, Sep 19, 2017 at 5:18 PM, Tsimbalist, Igor V > <igor.v.tsimbal...@intel.com> wrote: > >> -----Original Message----- > >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > >> ow...@gcc.gnu.org] On Behalf Of Uros Bizjak > >> Sent: Monday, September 18, 2017 12:17 PM > >> To: gcc-patches@gcc.gnu.org > >> Cc: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; Tsimbalist, > >> Igor V <igor.v.tsimbal...@intel.com> > >> Subject: Re: 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation > >> > >> Hello! > >> > >> > gcc/testsuite/ > >> > > >> > * g++.dg/cet-notrack-1.C: New test. > >> > * gcc.target/i386/cet-intrin-1.c: Likewise. > >> > * gcc.target/i386/cet-intrin-10.c: Likewise. > >> > * gcc.target/i386/cet-intrin-2.c: Likewise. > >> > * gcc.target/i386/cet-intrin-3.c: Likewise. > >> > * gcc.target/i386/cet-intrin-4.c: Likewise. > >> > * gcc.target/i386/cet-intrin-5.c: Likewise. > >> > * gcc.target/i386/cet-intrin-6.c: Likewise. > >> > * gcc.target/i386/cet-intrin-7.c: Likewise. > >> > * gcc.target/i386/cet-intrin-8.c: Likewise. > >> > * gcc.target/i386/cet-intrin-9.c: Likewise. > >> > * gcc.target/i386/cet-label.c: Likewise. > >> > * gcc.target/i386/cet-notrack-1a.c: Likewise. > >> > * gcc.target/i386/cet-notrack-1b.c: Likewise. > >> > * gcc.target/i386/cet-notrack-2a.c: Likewise. > >> > * gcc.target/i386/cet-notrack-2b.c: Likewise. > >> > * gcc.target/i386/cet-notrack-3.c: Likewise. > >> > * gcc.target/i386/cet-notrack-4a.c: Likewise. > >> > * gcc.target/i386/cet-notrack-4b.c: Likewise. > >> > * gcc.target/i386/cet-notrack-5a.c: Likewise. > >> > * gcc.target/i386/cet-notrack-5b.c: Likewise. > >> > * gcc.target/i386/cet-notrack-6a.c: Likewise. > >> > * gcc.target/i386/cet-notrack-6b.c: Likewise. > >> > * gcc.target/i386/cet-notrack-7.c: Likewise. > >> > * gcc.target/i386/cet-property-1.c: Likewise. > >> > * gcc.target/i386/cet-property-2.c: Likewise. > >> > * gcc.target/i386/cet-rdssp-1.c: Likewise. > >> > * gcc.target/i386/cet-sjlj-1.c: Likewise. > >> > * gcc.target/i386/cet-sjlj-2.c: Likewise. > >> > * gcc.target/i386/cet-sjlj-3.c: Likewise. > >> > * gcc.target/i386/cet-switch-1.c: Likewise. > >> > * gcc.target/i386/cet-switch-2.c: Likewise. > >> > * lib/target-supports.exp (check_effective_target_cet): New proc. > >> > >> A couple of questions: > >> > >> +/* { dg-do compile } */ > >> +/* { dg-options "-O2 -mcet" } */ > >> +/* { dg-final { scan-assembler-times "setssbsy" 2 } } */ > >> + > >> +#include <immintrin.h> > >> + > >> +void f1 (void) > >> +{ > >> + __builtin_ia32_setssbsy (); > >> +} > >> + > >> +void f2 (void) > >> +{ > >> + _setssbsy (); > >> +} > >> > >> Is there a reason that both, __builtin and intrinsic versions are > >> tested in a couple of places? The intrinsic version is just a wrapper > >> for __builtin, so IMO testing intrinsic version should be enough. > > No strong reason. Just to check that intrinsic names are recognized and > processed correctly. > > The implementation could change and the test will catch inconsistency. > > I would also assume a user will use intrinsics that's why I add intrinsic > > check. > Should I remove it? > > Actually, these __builtins are considered as implementation detail, and their > use should be discouraged. They are deliberately not documented, and users > should use intrinsic headers instead. That said, builtins won't change without > a reason, since Ada needs them. > > It can happen that the test fails due to change of intrinsics, so I'd > recommend > to remove them. Ok, I will remove intrinsic.
> >> diff --git a/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> new file mode 100644 > >> index 0000000..f9223a5 > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.target/i386/cet-rdssp-1.c > >> @@ -0,0 +1,39 @@ > >> +/* { dg-do run { target cet } } */ > >> +/* { dg-options "-O2 -finstrument-control-flow -mcet" } */ > >> > >> The "target cet" directive just checks that CET instructions can be > compiled. > >> The test will (probably?) fail on targets with binutils that can > >> compile CET instructions, but the target itself doesn't support CET. > >> If this is the case, then check header has to be introduced, so the > >> test can be bypassed on targets without runtime support. > > The test will not fail even if a target doesn't support CET as 'rdssp' > > instruction is a NOP on such target and further usage of CET > > instruction is bypassed. In this case the code > > > > + ssp = rdssp (ssp); > > > > Will keep ssp as 0. > > I assume that this is true also for other runtime tests, and this clears my > concern about runtime failures with updated binutils. Yes, that's true for other runtime tests. Igor > > Uros.