On Thu, Oct 12, 2017 at 8:54 PM, Tsimbalist, Igor V
<igor.v.tsimbal...@intel.com> wrote:
> Attached is an updated patch according to your comments. New tests are
> added to test ICF optimization in presence of nocf_check attribute.
--- a/gcc/testsuite/c-c++-common/fcf-protection-2.c
+++ b/gcc/testsuite/c-c++-common/fcf-protection-2.c
@@ -1,4 +1,4 @@
/* { dg-do compile } */
/* { dg-options "-fcf-protection=branch" } */
-/* { dg-error "'-fcf-protection=branch' is not supported for this
target" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */
+/* { dg-error "'-fcf-protection=branch' requires CET support on this
target. Use -mcet or one of -mibt, -mshstk options to enable CET" "" {
target { "i?86-*-* x86_64-*-*" } } 0 } */
Checking for "-fcf-protection=branch' requires CET support on this
target" should be enough. No need to check the whole message here and
in other tests.
/* { dg-error "'-fcf-protection=branch' is not supported for this
target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-3.c
b/gcc/testsuite/c-c++-common/fcf-protection-3.c
--- a/gcc/testsuite/c-c++-common/fcf-protection-4.c
+++ b/gcc/testsuite/c-c++-common/fcf-protection-4.c
@@ -1,4 +1,4 @@
/* { dg-do compile } */
/* { dg-options "-fcf-protection=none" } */
-/* { dg-bogus "'-fcf-protection=none' is not supported for this
target" "" { target { "i?86-*-* x86_64-*-*" } } 0 } */
+/* { dg-bogus "'-fcf-protection=none' res CET support on this target.
Use -mcet or one of -mibt, -mshstk options to enable CET" "" { target
{ "i?86-*-* x86_64-*-*" } } 0 } */
/* { dg-bogus "'-fcf-protection=none' is not supported for this
target" "" { target { ! "i?86-*-* x86_64-*-*" } } 0 } */
diff --git a/gcc/testsuite/c-c++-common/fcf-protection-5.c
b/gcc/testsuite/c-c++-common/fcf-protection-5.c
The above test checks for bogus messages? -fcf-protection=none option
should not generate any messages. So, the test should check that
-fcf-protection=none doesn't generate any error. (And, there is a typo
in the message, /s/res/requires.)
Uros.
> Igor
>
>
>> -----Original Message-----
>> From: Tsimbalist, Igor V
>> Sent: Tuesday, September 19, 2017 11:30 PM
>> To: Uros Bizjak <ubiz...@gmail.com>
>> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
>> <igor.v.tsimbal...@intel.com>
>> Subject: RE: 0006-Part-6.-Add-x86-tests-for-Intel-CET-implementation
>>
>> > -----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.
