Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Segher Boessenkool]

On Thu, Nov 08, 2018 at 06:30:21PM +0100, Uros Bizjak wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/ptwrite2.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mptwrite " } */
> +/* { dg-final { scan-assembler "ptwrite.*r" } } */
> +/* { dg-final { scan-assembler "ptwrite.*e" } } */
> 
> Better use \[^\n\r\] instead of .* to avoid unwanted multi-line matches.

Or better, write it as

/* { dg-final { scan-assembler {(?n)ptwrite.*r} } } */


Segher

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Uros Bizjak]

On Thu, Nov 8, 2018 at 7:03 PM Andi Kleen  wrote:
>
> > OK for x86 part (that is only PATCH 1/3). It looks that this part can
> > go to mainline as an independent patch from other patches in serie.
>
> Thanks.
>
> Note even 2/3 has a small i386 specific part. Would be good if you
> could take a look at that part.

It is "only" a hook (LGTM, BTW), but as part of functional part of the
patch, you will need an approval for the design and implementation of
the middle-end functionality first.

Uros.

> -Andi

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Andi Kleen]

> OK for x86 part (that is only PATCH 1/3). It looks that this part can
> go to mainline as an independent patch from other patches in serie.

Thanks.

Note even 2/3 has a small i386 specific part. Would be good if you
could take a look at that part.

-Andi

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Uros Bizjak]

Hello!

> From: Andi Kleen 
>
> Add builtins/intrinsics for PTWRITE. PTWRITE is a new instruction on Intel 
> Gemini Lake/
> Goldmont Plus that allows to write values into the Processor Trace log. This 
> allows
> very light weight instrumentation of programs.
>
> The intrinsics are compatible to icc. Automatically enabled for Goldmont Plus.
>
> gcc/:
>
> 2018-11-03  Andi Kleen  
>
> * common/config/i386/i386-common.c (OPTION_MASK_ISA_PTWRITE_SET): New.
> (OPTION_MASK_ISA_PTWRITE_UNSET): New.
> (ix86_handle_option): Handle OPT_mptwrite.
> * config/i386/cpuid.h (bit_PTWRITE): Add.
> * config/i386/driver-i386.c (host_detect_local_cpu): Detect ptwrite.
> * config/i386/i386-builtin.def (BDESC): Add ptwrite32/64.
> * config/i386/i386-c.c (ix86_target_macros_internal): Define __PTWRITE__.
> * config/i386/i386.c (ix86_target_string): Handle ptwrite.
> (ix86_option_override_internal): Handle PTA_PTWRITE.
> (ix86_valid_target_attribute_inner_p): Define ptwrite.
> (def_builtin2): Force UINT64 to be 64bit only.
> * config/i386/i386.h (TARGET_PTWRITE): Add.
> (TARGET_PTWRITE_P): Add.
> (PTA_PTWRITE): Add.
> * config/i386/i386.md: Define ptwrite.
> * config/i386/i386.opt: Add -mptwrite.
> * config/i386/immintrin.h (_ptwrite64): Add.
> (_ptwrite32): Add
> * doc/extend.texi: Document __builtin_ia32_ptwrite*.
> * doc/invoke.texi: Document -mptwrite.
>
> gcc/testsuite/ChangeLog:
>
> 2018-11-03  Andi Kleen  
>
> * gcc.target/i386/ptwrite1.c: New test.
> * gcc.target/i386/ptwrite2.c: New test.

OK for x86 part (that is only PATCH 1/3). It looks that this part can
go to mainline as an independent patch from other patches in serie.

diff --git a/gcc/testsuite/gcc.target/i386/ptwrite2.c
b/gcc/testsuite/gcc.target/i386/ptwrite2.c
new file mode 100644
index 000..299c6511ef4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/ptwrite2.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mptwrite " } */
+/* { dg-final { scan-assembler "ptwrite.*r" } } */
+/* { dg-final { scan-assembler "ptwrite.*e" } } */

Better use \[^\n\r\] instead of .* to avoid unwanted multi-line matches.

Thanks,
Uros.

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Andi Kleen]

Andi Kleen  writes:

Ping!

> From: Andi Kleen 
>
> Add builtins/intrinsics for PTWRITE. PTWRITE is a new instruction on Intel 
> Gemini Lake/
> Goldmont Plus that allows to write values into the Processor Trace log. This 
> allows
> very light weight instrumentation of programs.
>
> The intrinsics are compatible to icc. Automatically enabled for Goldmont Plus.
>
> gcc/:
>
> 2018-11-03  Andi Kleen  
>
>   * common/config/i386/i386-common.c (OPTION_MASK_ISA_PTWRITE_SET): New.
>   (OPTION_MASK_ISA_PTWRITE_UNSET): New.
>   (ix86_handle_option): Handle OPT_mptwrite.
>   * config/i386/cpuid.h (bit_PTWRITE): Add.
>   * config/i386/driver-i386.c (host_detect_local_cpu): Detect ptwrite.
>   * config/i386/i386-builtin.def (BDESC): Add ptwrite32/64.
>   * config/i386/i386-c.c (ix86_target_macros_internal): Define 
> __PTWRITE__.
>   * config/i386/i386.c (ix86_target_string): Handle ptwrite.
>   (ix86_option_override_internal): Handle PTA_PTWRITE.
>   (ix86_valid_target_attribute_inner_p): Define ptwrite.
>   (def_builtin2): Force UINT64 to be 64bit only.
>   * config/i386/i386.h (TARGET_PTWRITE): Add.
>   (TARGET_PTWRITE_P): Add.
>   (PTA_PTWRITE): Add.
>   * config/i386/i386.md: Define ptwrite.
>   * config/i386/i386.opt: Add -mptwrite.
>   * config/i386/immintrin.h (_ptwrite64): Add.
>   (_ptwrite32): Add
>   * doc/extend.texi: Document __builtin_ia32_ptwrite*.
>   * doc/invoke.texi: Document -mptwrite.
>
> gcc/testsuite/ChangeLog:
>
> 2018-11-03  Andi Kleen  
>
>   * gcc.target/i386/ptwrite1.c: New test.
>   * gcc.target/i386/ptwrite2.c: New test.
> ---
>  gcc/common/config/i386/i386-common.c | 15 
>  gcc/config/i386/cpuid.h  |  4 
>  gcc/config/i386/driver-i386.c| 12 ++
>  gcc/config/i386/i386-builtin.def |  4 
>  gcc/config/i386/i386-c.c |  2 ++
>  gcc/config/i386/i386.c   |  9 ++-
>  gcc/config/i386/i386.h   |  5 +++-
>  gcc/config/i386/i386.md  | 10 
>  gcc/config/i386/i386.opt |  4 
>  gcc/config/i386/immintrin.h  | 26 
>  gcc/doc/extend.texi  |  9 +++
>  gcc/doc/invoke.texi  |  7 --
>  gcc/testsuite/gcc.target/i386/ptwrite1.c | 30 
>  gcc/testsuite/gcc.target/i386/ptwrite2.c | 14 +++
>  14 files changed, 147 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/ptwrite1.c
>  create mode 100644 gcc/testsuite/gcc.target/i386/ptwrite2.c
>
> diff --git a/gcc/common/config/i386/i386-common.c 
> b/gcc/common/config/i386/i386-common.c
> index f12806ef3a9..f740995c1e4 100644
> --- a/gcc/common/config/i386/i386-common.c
> +++ b/gcc/common/config/i386/i386-common.c
> @@ -140,6 +140,7 @@ along with GCC; see the file COPYING3.  If not see
>  
>  #define OPTION_MASK_ISA_FSGSBASE_SET OPTION_MASK_ISA_FSGSBASE
>  #define OPTION_MASK_ISA_RDRND_SET OPTION_MASK_ISA_RDRND
> +#define OPTION_MASK_ISA_PTWRITE_SET OPTION_MASK_ISA_PTWRITE
>  #define OPTION_MASK_ISA_F16C_SET \
>(OPTION_MASK_ISA_F16C | OPTION_MASK_ISA_AVX_SET)
>  #define OPTION_MASK_ISA_MWAITX_SET OPTION_MASK_ISA_MWAITX
> @@ -267,6 +268,7 @@ along with GCC; see the file COPYING3.  If not see
>  
>  #define OPTION_MASK_ISA_FSGSBASE_UNSET OPTION_MASK_ISA_FSGSBASE
>  #define OPTION_MASK_ISA_RDRND_UNSET OPTION_MASK_ISA_RDRND
> +#define OPTION_MASK_ISA_PTWRITE_UNSET OPTION_MASK_ISA_PTWRITE
>  #define OPTION_MASK_ISA_F16C_UNSET OPTION_MASK_ISA_F16C
>  
>  #define OPTION_MASK_ISA_GENERAL_REGS_ONLY_UNSET \
> @@ -1125,6 +1127,19 @@ ix86_handle_option (struct gcc_options *opts,
>   }
>return true;
>  
> +case OPT_mptwrite:
> +  if (value)
> + {
> +   opts->x_ix86_isa_flags2 |= OPTION_MASK_ISA_PTWRITE_SET;
> +   opts->x_ix86_isa_flags2_explicit |= OPTION_MASK_ISA_PTWRITE_SET;
> + }
> +  else
> + {
> +   opts->x_ix86_isa_flags2 &= ~OPTION_MASK_ISA_PTWRITE_UNSET;
> +   opts->x_ix86_isa_flags2_explicit |= OPTION_MASK_ISA_PTWRITE_UNSET;
> + }
> +  return true;
> +
>  case OPT_mf16c:
>if (value)
>   {
> diff --git a/gcc/config/i386/cpuid.h b/gcc/config/i386/cpuid.h
> index 7e9e2d153dc..2e6d4a55602 100644
> --- a/gcc/config/i386/cpuid.h
> +++ b/gcc/config/i386/cpuid.h
> @@ -126,6 +126,10 @@
>  #define bit_XSAVEC   (1 << 1)
>  #define bit_XSAVES   (1 << 3)
>  
> +/* PT sub leaf (%eax == 14, %ecx == 0) */
> +/* %ebx */
> +#define bit_PTWRITE  (1 << 4)
> +
>  /* Signatures for different CPU implementations as returned in uses
> of cpuid with level 0.  */
>  #define signature_AMD_ebx0x68747541
> diff --git a/gcc/config/i386/driver-i386.c b/gcc/config/i386/driver-i386.c
> index 8c830bde1dd..423b1c3827f 100644
> --- a/gcc/config/i386/driver-i386.c
> +++ b/gcc/config/i386/driver-i386.c
> @@ -427,6 +427,8 @@ const char 

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Uros Bizjak]

On Fri, May 11, 2018 at 2:44 AM, Andi Kleen  wrote:
>> @@ -31325,7 +31329,21 @@ ix86_init_mmx_sse_builtins (void)
>>  continue;
>>
>>ftype = (enum ix86_builtin_func_type) d->flag;
>> -  def_builtin2 (d->mask, d->name, ftype, d->code);
>> +  decl = def_builtin2 (d->mask, d->name, ftype, d->code);
>> +
>> +  /* Avoid edges for ptwrites generated by vartrace pass.  */
>> +  if (decl)
>> +{
>> +  DECL_ATTRIBUTES (decl) = build_tree_list (get_identifier ("leaf"),
>> +NULL_TREE);
>> +  TREE_NOTHROW (decl) = 1;
>> +}
>> +  else
>> +{
>> +  ix86_builtins_isa[(int)d->code].leaf_p = true;
>> +  ix86_builtins_isa[(int)d->code].nothrow_p = true;
>> +}
>> +
>>
>> Can you please explain what is the purpose of the above change?
>
> With the vartrace patch, which was the next patch in the original
> patchkit, the compiler can generate a lot of ptwrite builtins,
> and adding so many edges can slow it down. I originally copied
> this from the MPX builtins (which had the same problem).

Please add new def_builtin_noedge2 (or something appropriately named)
aux function and declare these two new builtins manually in
ix86_init_mmx_sse_builtins.

Uros.

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Andi Kleen]

> @@ -31325,7 +31329,21 @@ ix86_init_mmx_sse_builtins (void)
>  continue;
> 
>ftype = (enum ix86_builtin_func_type) d->flag;
> -  def_builtin2 (d->mask, d->name, ftype, d->code);
> +  decl = def_builtin2 (d->mask, d->name, ftype, d->code);
> +
> +  /* Avoid edges for ptwrites generated by vartrace pass.  */
> +  if (decl)
> +{
> +  DECL_ATTRIBUTES (decl) = build_tree_list (get_identifier ("leaf"),
> +NULL_TREE);
> +  TREE_NOTHROW (decl) = 1;
> +}
> +  else
> +{
> +  ix86_builtins_isa[(int)d->code].leaf_p = true;
> +  ix86_builtins_isa[(int)d->code].nothrow_p = true;
> +}
> +
> 
> Can you please explain what is the purpose of the above change?

With the vartrace patch, which was the next patch in the original
patchkit, the compiler can generate a lot of ptwrite builtins,
and adding so many edges can slow it down. I originally copied
this from the MPX builtins (which had the same problem).

Possibly it could be in another patch, but then it would seem 
better to have the same semantics always. FWIW I don't think
the edges are needed for anything, but of course for moderate
use of the builtin it doesn't really matter.

-Andi

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Uros Bizjak]

On Wed, May 9, 2018 at 1:23 PM, Peryt, Sebastian
 wrote:
> I have rebased this patch to the latest trunk and addressed comments. Also, 
> there was a test in changelog,
> but not in the patch itself - this has been added.
>
> Is it ok for trunk and backport to GCC-8 after few days?
>
> gcc/
>
> * common/config/i386/i386-common.c (OPTION_MASK_ISA_PTWRITE_SET,
> OPTION_MASK_ISA_PTWRITE_UNSET): New.
> (ix86_handle_option): Handle OPT_mptwrite.
> * config/i386/cpuid.h (bit_PTWRITE): Add.
> * config/i386/driver-i386.c (host_detect_local_cpu): Detect
> PTWRITE CPUID.
> * config/i386/i386-builtin.def (PTWRITE): Add PTWRITE.
> * config/i386/i386-c.c (ix86_target_macros_internal):
> Support __PTWRITE__.
> * config/i386/i386.c (ix86_target_string): Add -mptwrite.
> (ix86_valid_target_attribute_inner_p): Support ptwrite.
> (ix86_init_mmx_sse_builtins): Add edges detection for ptwrites
> generated by vartrace.
> * config/i386/i386.h (TARGET_PTWRITE): Add.
> (TARGET_PTWRITE_P): Add.
> * config/i386/i386.md: Add ptwrite.
> * config/i386/i386.opt: Add -mptwrite.
> * config/i386/immintrin.h (target):
> (_ptwrite64): Add.
> (_ptwrite32): Add.
> * doc/extend.texi: Document ptwrite builtins.
> * doc/invoke.texi: Document -mptwrite.
>
> gcc/testsuite/
>
> * gcc.target/i386/ptwrite-1.c: New test.

@@ -31325,7 +31329,21 @@ ix86_init_mmx_sse_builtins (void)
 continue;

   ftype = (enum ix86_builtin_func_type) d->flag;
-  def_builtin2 (d->mask, d->name, ftype, d->code);
+  decl = def_builtin2 (d->mask, d->name, ftype, d->code);
+
+  /* Avoid edges for ptwrites generated by vartrace pass.  */
+  if (decl)
+{
+  DECL_ATTRIBUTES (decl) = build_tree_list (get_identifier ("leaf"),
+NULL_TREE);
+  TREE_NOTHROW (decl) = 1;
+}
+  else
+{
+  ix86_builtins_isa[(int)d->code].leaf_p = true;
+  ix86_builtins_isa[(int)d->code].nothrow_p = true;
+}
+

Can you please explain what is the purpose of the above change?

Uros.

RE: [PATCH 1/3] Add PTWRITE builtins for x86

[Peryt, Sebastian]

I have rebased this patch to the latest trunk and addressed comments. Also, 
there was a test in changelog,
but not in the patch itself - this has been added.

Is it ok for trunk and backport to GCC-8 after few days?

gcc/

* common/config/i386/i386-common.c (OPTION_MASK_ISA_PTWRITE_SET,
OPTION_MASK_ISA_PTWRITE_UNSET): New.
(ix86_handle_option): Handle OPT_mptwrite.
* config/i386/cpuid.h (bit_PTWRITE): Add.
* config/i386/driver-i386.c (host_detect_local_cpu): Detect
PTWRITE CPUID.
* config/i386/i386-builtin.def (PTWRITE): Add PTWRITE.
* config/i386/i386-c.c (ix86_target_macros_internal):
Support __PTWRITE__.
* config/i386/i386.c (ix86_target_string): Add -mptwrite.
(ix86_valid_target_attribute_inner_p): Support ptwrite.
(ix86_init_mmx_sse_builtins): Add edges detection for ptwrites
generated by vartrace.
* config/i386/i386.h (TARGET_PTWRITE): Add.
(TARGET_PTWRITE_P): Add.
* config/i386/i386.md: Add ptwrite.
* config/i386/i386.opt: Add -mptwrite.
* config/i386/immintrin.h (target):
(_ptwrite64): Add.
(_ptwrite32): Add.
* doc/extend.texi: Document ptwrite builtins.
* doc/invoke.texi: Document -mptwrite.

gcc/testsuite/

* gcc.target/i386/ptwrite-1.c: New test.

Sebastian


> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Andi Kleen
> Sent: Monday, February 12, 2018 3:53 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Metzger, Markus T ; ubiz...@gmail.com;
> Andi Kleen 
> Subject: [PATCH 1/3] Add PTWRITE builtins for x86
> 
> From: Andi Kleen 
> 
> Add builtins/intrinsics for PTWRITE. PTWRITE is a new instruction on Intel 
> Cherry
> Trail that allows to write values into the Processor Trace log.
> 
> This is fairly straight forward, except I had to add isa2 support for variable
> number of operands.
> 
> gcc/:
> 
> 2018-02-10  Andi Kleen  
> 
>   * common/config/i386/i386-common.c
> (OPTION_MASK_ISA_PTWRITE_SET):
>   (OPTION_MASK_ISA_PTWRITE_UNSET): New.
>   (ix86_handle_option): Handle OPT_mptwrite.
>   * config/i386/cpuid.h (bit_PTWRITE): Add.
>   * config/i386/driver-i386.c (host_detect_local_cpu): Detect
>   PTWRITE CPUID.
>   * config/i386/i386-builtin.def (PTWRITE): Add PTWRITE.
>   * config/i386/i386-c.c (ix86_target_macros_internal):
>   Support __PTWRITE__.
>   * config/i386/i386.c (ix86_target_string): Add -mptwrite.
>   (ix86_valid_target_attribute_inner_p): Support ptwrite.
>   (BDESC_VERIFYS): Verify SPECIAL_ARGS2.
>   (ix86_init_mmx_sse_builtins): Handle special args2.
>   * config/i386/i386.h (TARGET_PTWRITE): Add.
>   (TARGET_PTWRITE_P): Add.
>   * config/i386/i386.md: Add ptwrite.
>   * config/i386/i386.opt: Add -mptwrite.
>   * config/i386/immintrin.h (target):
>   (_ptwrite_u64): Add.
>   (_ptwrite_u32): Add.
>   * doc/extend.texi: Document ptwrite builtins.
>   * doc/invoke.texi: Document -mptwrite.
> 
> gcc/testsuite/:
> 
> 2018-02-10  Andi Kleen  
> 
>   * gcc.target/i386/ptwrite1.c: New test.
>   * gcc.target/i386/ptwrite2.c: New test.


0001-PTWRITE-intrinsics.patch
Description: 0001-PTWRITE-intrinsics.patch

Re: [PATCH 1/3] Add PTWRITE builtins for x86

[Joseph Myers]

On Sun, 11 Feb 2018, Andi Kleen wrote:

> @@ -27064,6 +27064,9 @@ preferred alignment to 
> @option{-mpreferred-stack-boundary=2}.
>  @itemx -mfsgsbase
>  @opindex mfsgsbase
>  @need 200
> +@itemx -mptwrite
> +@opindex mptwrite
> +@need 200
>  @itemx -mrdrnd
>  @opindex mrdrnd
>  @need 200

This @itemx sequence is above a paragraph that lists the corresponding 
instruction set extension for each option.  You need to insert an 
appropriate entry in that list between "FSGSBASE, RDRND".

-- 
Joseph S. Myers
jos...@codesourcery.com