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.
>> 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.
Uros.
