public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
@ 2015-03-09 22:15 Martin Liška
  2015-03-13  4:43 ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Liška @ 2015-03-09 22:15 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

Hello.

During compilation of Chromium project, I noticed (perf top) that ix86_valid_target_attribute is utilized by about 0.8-1.9%.
Following patch introduces simple optimization that reduces the utilization to ~0.05%.

Tests have been running on x86_64-linux-pc.

Ready for trunk?
Martin

[-- Attachment #2: 0001-def_builtin_const-speed-up.patch --]
[-- Type: text/x-patch, Size: 1884 bytes --]

From d20232455993882e4b7f769725a0c1589747b8a3 Mon Sep 17 00:00:00 2001
From: marxin <marxin.liska@gmail.com>
Date: Sun, 8 Mar 2015 19:39:55 -0500
Subject: [PATCH] def_builtin_const: speed-up.

gcc/ChangeLog:

2015-03-09  Martin Liska  <marxin.liska@gmail.com>

	* config/i386/i386.c (def_builtin): Collect union of all
	possible masks.
	(ix86_add_new_builtins): Do not iterate over all builtins
	in cases that isa value has no intersection with possible masks
	and(or) last passed value is equal to the provided.
---
 gcc/config/i386/i386.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ab8f03a..5f180b6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -30592,6 +30592,8 @@ struct builtin_isa {
 
 static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
 
+/* Union of all masks that are part of builtin_isa structures.  */
+static HOST_WIDE_INT defined_isa_values = 0;
 
 /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
    of which isa_flags to use in the ix86_builtins_isa array.  Stores the
@@ -30619,6 +30621,7 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
   if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
     {
       ix86_builtins_isa[(int) code].isa = mask;
+      defined_isa_values |= mask;
 
       mask &= ~OPTION_MASK_ISA_64BIT;
       if (mask == 0
@@ -30670,6 +30673,14 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
 static void
 ix86_add_new_builtins (HOST_WIDE_INT isa)
 {
+  /* Last cached isa value.  */
+  static HOST_WIDE_INT last_tested_isa_value = 0;
+
+  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)
+    return;
+
+  last_tested_isa_value = isa;
+
   int i;
   tree saved_current_target_pragma = current_target_pragma;
   current_target_pragma = NULL_TREE;
-- 
2.1.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-09 22:15 [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute) Martin Liška
@ 2015-03-13  4:43 ` Jan Hubicka
  2015-03-19 21:21   ` Martin Liška
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2015-03-13  4:43 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches, Jan Hubicka

> 2015-03-09  Martin Liska  <marxin.liska@gmail.com>
> 
> 	* config/i386/i386.c (def_builtin): Collect union of all
> 	possible masks.
> 	(ix86_add_new_builtins): Do not iterate over all builtins
> 	in cases that isa value has no intersection with possible masks
> 	and(or) last passed value is equal to the provided.
> ---
>  gcc/config/i386/i386.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index ab8f03a..5f180b6 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -30592,6 +30592,8 @@ struct builtin_isa {
>  
>  static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
>  
> +/* Union of all masks that are part of builtin_isa structures.  */
> +static HOST_WIDE_INT defined_isa_values = 0;
>  
>  /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
>     of which isa_flags to use in the ix86_builtins_isa array.  Stores the
> @@ -30619,6 +30621,7 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
>    if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
>      {
>        ix86_builtins_isa[(int) code].isa = mask;
> +      defined_isa_values |= mask;

I think you can move this down to set_and_not_build_p set.  Please add also
comment explaining the caching mehanism.
>  
>        mask &= ~OPTION_MASK_ISA_64BIT;
>        if (mask == 0
> @@ -30670,6 +30673,14 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>  static void
>  ix86_add_new_builtins (HOST_WIDE_INT isa)
>  {
> +  /* Last cached isa value.  */
> +  static HOST_WIDE_INT last_tested_isa_value = 0;
> +
> +  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)

Heer you need to compare (isa & defined_isa_values) == (isa &
last_tested_isa_value) right, because we have isa flags that enable no
builtins.

Honza

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-13  4:43 ` Jan Hubicka
@ 2015-03-19 21:21   ` Martin Liška
  2015-03-20  0:10     ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Liška @ 2015-03-19 21:21 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]

On 03/13/2015 05:42 AM, Jan Hubicka wrote:
>> 2015-03-09  Martin Liska  <marxin.liska@gmail.com>
>>
>> 	* config/i386/i386.c (def_builtin): Collect union of all
>> 	possible masks.
>> 	(ix86_add_new_builtins): Do not iterate over all builtins
>> 	in cases that isa value has no intersection with possible masks
>> 	and(or) last passed value is equal to the provided.
>> ---
>>   gcc/config/i386/i386.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index ab8f03a..5f180b6 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -30592,6 +30592,8 @@ struct builtin_isa {
>>
>>   static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
>>
>> +/* Union of all masks that are part of builtin_isa structures.  */
>> +static HOST_WIDE_INT defined_isa_values = 0;
>>
>>   /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
>>      of which isa_flags to use in the ix86_builtins_isa array.  Stores the
>> @@ -30619,6 +30621,7 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
>>     if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
>>       {
>>         ix86_builtins_isa[(int) code].isa = mask;
>> +      defined_isa_values |= mask;
>
> I think you can move this down to set_and_not_build_p set.  Please add also
> comment explaining the caching mehanism.

Hi.

Explanation of the patch is introduced.

>>
>>         mask &= ~OPTION_MASK_ISA_64BIT;
>>         if (mask == 0
>> @@ -30670,6 +30673,14 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>>   static void
>>   ix86_add_new_builtins (HOST_WIDE_INT isa)
>>   {
>> +  /* Last cached isa value.  */
>> +  static HOST_WIDE_INT last_tested_isa_value = 0;
>> +
>> +  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)
>
> Heer you need to compare (isa & defined_isa_values) == (isa &
> last_tested_isa_value) right, because we have isa flags that enable no
> builtins.

I do not understand why, the guard simply ignores last value, which is already processed and
'isa' with any intersection with defined_isa_values. Maybe I miss something?

Thanks,
Martin

>
> Honza
>


[-- Attachment #2: 0001-def_builtin_const-speed-up.patch --]
[-- Type: text/x-patch, Size: 2097 bytes --]

From f40f0fa07d8a9643050c30cd4e29c8c4c8de6cce Mon Sep 17 00:00:00 2001
From: marxin <marxin.liska@gmail.com>
Date: Sun, 8 Mar 2015 19:39:55 -0500
Subject: [PATCH] def_builtin_const: speed-up.

gcc/ChangeLog:

2015-03-09  Martin Liska  <mliska@suse.cz>

	* config/i386/i386.c (def_builtin): Collect union of all
	possible masks.
	(ix86_add_new_builtins): Do not iterate over all builtins
	in cases that isa value has no intersection with possible masks
	and(or) last passed value is equal to the provided.
---
 gcc/config/i386/i386.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ab8f03a..134b349 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -30592,6 +30592,8 @@ struct builtin_isa {
 
 static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
 
+/* Union of all masks that are part of builtin_isa structures.  */
+static HOST_WIDE_INT defined_isa_values = 0;
 
 /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
    of which isa_flags to use in the ix86_builtins_isa array.  Stores the
@@ -30619,6 +30621,7 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
   if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
     {
       ix86_builtins_isa[(int) code].isa = mask;
+      defined_isa_values |= mask;
 
       mask &= ~OPTION_MASK_ISA_64BIT;
       if (mask == 0
@@ -30670,6 +30673,17 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
 static void
 ix86_add_new_builtins (HOST_WIDE_INT isa)
 {
+  /* Last cached isa value.  */
+  static HOST_WIDE_INT last_tested_isa_value = 0;
+
+  /* We iterate through all defined builtins just if the last tested
+     values is different from ISA and just in case there is any intersection
+     between ISA value and union of all possible configurations.  */
+  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)
+    return;
+
+  last_tested_isa_value = isa;
+
   int i;
   tree saved_current_target_pragma = current_target_pragma;
   current_target_pragma = NULL_TREE;
-- 
2.1.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-19 21:21   ` Martin Liška
@ 2015-03-20  0:10     ` Jan Hubicka
  2015-03-20 10:33       ` Martin Liška
  0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2015-03-20  0:10 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches, Jan Hubicka

> Hi.
> 
> Explanation of the patch is introduced.

thanks
> 
> >>
> >>        mask &= ~OPTION_MASK_ISA_64BIT;
> >>        if (mask == 0
> >>@@ -30670,6 +30673,14 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
> >>  static void
> >>  ix86_add_new_builtins (HOST_WIDE_INT isa)
> >>  {
> >>+  /* Last cached isa value.  */
> >>+  static HOST_WIDE_INT last_tested_isa_value = 0;
> >>+
> >>+  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)
> >
> >Heer you need to compare (isa & defined_isa_values) == (isa &
> >last_tested_isa_value) right, because we have isa flags that enable no
> >builtins.
> 
> I do not understand why, the guard simply ignores last value, which is already processed and
> 'isa' with any intersection with defined_isa_values. Maybe I miss something?

I think you can also skip processing in cases ISA changed but the change has empty intersection with
defined_isa_values.

Honza

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-20  0:10     ` Jan Hubicka
@ 2015-03-20 10:33       ` Martin Liška
  2015-03-20 11:25         ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Liška @ 2015-03-20 10:33 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

On 03/20/2015 01:10 AM, Jan Hubicka wrote:
>> Hi.
>>
>> Explanation of the patch is introduced.
>
> thanks
>>
>>>>
>>>>         mask &= ~OPTION_MASK_ISA_64BIT;
>>>>         if (mask == 0
>>>> @@ -30670,6 +30673,14 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>>>>   static void
>>>>   ix86_add_new_builtins (HOST_WIDE_INT isa)
>>>>   {
>>>> +  /* Last cached isa value.  */
>>>> +  static HOST_WIDE_INT last_tested_isa_value = 0;
>>>> +
>>>> +  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value)
>>>
>>> Heer you need to compare (isa & defined_isa_values) == (isa &
>>> last_tested_isa_value) right, because we have isa flags that enable no
>>> builtins.
>>
>> I do not understand why, the guard simply ignores last value, which is already processed and
>> 'isa' with any intersection with defined_isa_values. Maybe I miss something?
>
> I think you can also skip processing in cases ISA changed but the change has empty intersection with
> defined_isa_values.
>
> Honza
>

Good idea.

I've integrated this part to upgraded patch, which can survive regression tests on x86_64-linux-pc
and can bootstrap.

Ready for trunk?
Thanks,
Martin

[-- Attachment #2: 0001-def_builtin_const-speed-up.patch --]
[-- Type: text/x-patch, Size: 2293 bytes --]

From 5df909a7d06e91c9506a7d7186015b431f9db04a Mon Sep 17 00:00:00 2001
From: marxin <marxin.liska@gmail.com>
Date: Sun, 8 Mar 2015 19:39:55 -0500
Subject: [PATCH] def_builtin_const: speed-up.

gcc/ChangeLog:

2015-03-09  Martin Liska  <mliska@suse.cz>

	* config/i386/i386.c (def_builtin): Collect union of all
	possible masks.
	(ix86_add_new_builtins): Do not iterate over all builtins
	in cases that isa value has no intersection with possible masks
	and(or) last passed value is equal to the provided.
---
 gcc/config/i386/i386.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ab8f03a..afb2fa4 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -30592,6 +30592,8 @@ struct builtin_isa {
 
 static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
 
+/* Union of all masks that are part of builtin_isa structures.  */
+static HOST_WIDE_INT defined_isa_values = 0;
 
 /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
    of which isa_flags to use in the ix86_builtins_isa array.  Stores the
@@ -30619,6 +30621,7 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
   if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
     {
       ix86_builtins_isa[(int) code].isa = mask;
+      defined_isa_values |= mask;
 
       mask &= ~OPTION_MASK_ISA_64BIT;
       if (mask == 0
@@ -30670,6 +30673,20 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
 static void
 ix86_add_new_builtins (HOST_WIDE_INT isa)
 {
+  /* Last cached isa value.  */
+  static HOST_WIDE_INT last_tested_isa_value = 0;
+
+  /* We iterate through all defined builtins just if the last tested
+     values is different from ISA and just in case there is any intersection
+     between ISA value and union of all possible configurations.
+     Last condition skips iterations if ISA is changed by the change has
+     empty intersection with defined_isa_values.  */
+  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value
+       || ((isa ^ last_tested_isa_value) & defined_isa_values) == 0)
+    return;
+
+  last_tested_isa_value = isa;
+
   int i;
   tree saved_current_target_pragma = current_target_pragma;
   current_target_pragma = NULL_TREE;
-- 
2.1.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-20 10:33       ` Martin Liška
@ 2015-03-20 11:25         ` Jakub Jelinek
  2015-03-20 14:07           ` Martin Liška
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2015-03-20 11:25 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches, Jan Hubicka

On Fri, Mar 20, 2015 at 11:33:09AM +0100, Martin Liška wrote:
> @@ -30670,6 +30673,20 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>  static void
>  ix86_add_new_builtins (HOST_WIDE_INT isa)
>  {
> +  /* Last cached isa value.  */
> +  static HOST_WIDE_INT last_tested_isa_value = 0;
> +
> +  /* We iterate through all defined builtins just if the last tested
> +     values is different from ISA and just in case there is any intersection
> +     between ISA value and union of all possible configurations.
> +     Last condition skips iterations if ISA is changed by the change has
> +     empty intersection with defined_isa_values.  */
> +  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value
> +       || ((isa ^ last_tested_isa_value) & defined_isa_values) == 0)
> +    return;
> +
> +  last_tested_isa_value = isa;

Isn't at least the isa == last_tested_isa_value test useless?
I mean, if they are equal, then (isa ^ last_tested_isa_value) is 0
and so the third test is true.

Also, given that the loop does something only for
  (ix86_builtins_isa[i].isa & isa) != 0
it means once you call ix86_add_new_builtins with some particular
bit set in the isa, it doesn't make sense to try that bit again.

So, I think it would be better:
1) rename defined_isa_values bitmask to say deferred_isa_values
   (as in, isa bits that might still enable any new builtins)
2) in def_builtin, don't or in the mask unconditionally, but only
   in the else case - when add_builtin_function is not called
3) in ix86_add_new_builtins, don't add last_tested_isa_value at all, instead
   do:
  if ((isa & deferred_isa_values) == 0)
    return;

  deferred_isa_values &= ~isa;

That way, when you actually enable all builtins (either immediately in
def_builtin because it wasn't C/C++-like FE, or because all ISAs were
enabled from the start, or because ix86_add_new_builtins has been already
called with sufficiently full bitmask), deferred_isa_values will be 0 and
you won't do anything in the function any more.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-20 11:25         ` Jakub Jelinek
@ 2015-03-20 14:07           ` Martin Liška
  2015-03-20 17:50             ` Jan Hubicka
  0 siblings, 1 reply; 8+ messages in thread
From: Martin Liška @ 2015-03-20 14:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Jan Hubicka

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

On 03/20/2015 12:24 PM, Jakub Jelinek wrote:
> On Fri, Mar 20, 2015 at 11:33:09AM +0100, Martin Liška wrote:
>> @@ -30670,6 +30673,20 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>>   static void
>>   ix86_add_new_builtins (HOST_WIDE_INT isa)
>>   {
>> +  /* Last cached isa value.  */
>> +  static HOST_WIDE_INT last_tested_isa_value = 0;
>> +
>> +  /* We iterate through all defined builtins just if the last tested
>> +     values is different from ISA and just in case there is any intersection
>> +     between ISA value and union of all possible configurations.
>> +     Last condition skips iterations if ISA is changed by the change has
>> +     empty intersection with defined_isa_values.  */
>> +  if ((isa & defined_isa_values) == 0 || isa == last_tested_isa_value
>> +       || ((isa ^ last_tested_isa_value) & defined_isa_values) == 0)
>> +    return;
>> +
>> +  last_tested_isa_value = isa;
>
> Isn't at least the isa == last_tested_isa_value test useless?
> I mean, if they are equal, then (isa ^ last_tested_isa_value) is 0
> and so the third test is true.
>
> Also, given that the loop does something only for
>    (ix86_builtins_isa[i].isa & isa) != 0
> it means once you call ix86_add_new_builtins with some particular
> bit set in the isa, it doesn't make sense to try that bit again.
>
> So, I think it would be better:
> 1) rename defined_isa_values bitmask to say deferred_isa_values
>     (as in, isa bits that might still enable any new builtins)
> 2) in def_builtin, don't or in the mask unconditionally, but only
>     in the else case - when add_builtin_function is not called
> 3) in ix86_add_new_builtins, don't add last_tested_isa_value at all, instead
>     do:
>    if ((isa & deferred_isa_values) == 0)
>      return;
>
>    deferred_isa_values &= ~isa;
>
> That way, when you actually enable all builtins (either immediately in
> def_builtin because it wasn't C/C++-like FE, or because all ISAs were
> enabled from the start, or because ix86_add_new_builtins has been already
> called with sufficiently full bitmask), deferred_isa_values will be 0 and
> you won't do anything in the function any more.
>
> 	Jakub
>

Thank you Jakub for smart solution.
I've just implemented new patch, which I've been testing.

What do you think about it?
Martin

[-- Attachment #2: 0001-Speed-up-def_builtin_const-ix86_valid_target_attribu.patch --]
[-- Type: text/x-patch, Size: 1931 bytes --]

From 3469163018d3c8ad2849bb9d241e12ae8723da90 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Fri, 20 Mar 2015 14:51:47 +0100
Subject: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute).

gcc/ChangeLog:

2015-03-20  Martin Liska  <mliska@suse.cz>
	    Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (def_builtin): Set deferred_isa_values for
	masks that can potentially include a builtin.
	(ix86_add_new_builtins): Introduce fast filter for isa values
	that cannot trigger builtin inclusion.
---
 gcc/config/i386/i386.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 47deda7..82a4848 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -30588,6 +30588,8 @@ struct builtin_isa {
 
 static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
 
+/* Bits that can still enable any inclusion of a builtin.  */
+static HOST_WIDE_INT deferred_isa_values = 0;
 
 /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
    of which isa_flags to use in the ix86_builtins_isa array.  Stores the
@@ -30631,6 +30633,9 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
 	}
       else
 	{
+	  /* Just a MASK where set_and_not_built_p == true can potentially
+	     include a builtin.  */
+	  deferred_isa_values |= mask;
 	  ix86_builtins[(int) code] = NULL_TREE;
 	  ix86_builtins_isa[(int) code].tcode = tcode;
 	  ix86_builtins_isa[(int) code].name = name;
@@ -30666,6 +30671,12 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
 static void
 ix86_add_new_builtins (HOST_WIDE_INT isa)
 {
+  if ((isa & deferred_isa_values) == 0)
+    return;
+
+  /* Bits in ISA value can be removed from potential isa values.  */
+  deferred_isa_values &= ~isa;
+
   int i;
   tree saved_current_target_pragma = current_target_pragma;
   current_target_pragma = NULL_TREE;
-- 
2.1.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute)
  2015-03-20 14:07           ` Martin Liška
@ 2015-03-20 17:50             ` Jan Hubicka
  0 siblings, 0 replies; 8+ messages in thread
From: Jan Hubicka @ 2015-03-20 17:50 UTC (permalink / raw)
  To: Martin Liška; +Cc: Jakub Jelinek, gcc-patches, Jan Hubicka

> gcc/ChangeLog:
> 
> 2015-03-20  Martin Liska  <mliska@suse.cz>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	* config/i386/i386.c (def_builtin): Set deferred_isa_values for
> 	masks that can potentially include a builtin.
> 	(ix86_add_new_builtins): Introduce fast filter for isa values
> 	that cannot trigger builtin inclusion.

Thanks, this looks better.
Patch is OK if it passes testing.

Honza
> ---
>  gcc/config/i386/i386.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 47deda7..82a4848 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -30588,6 +30588,8 @@ struct builtin_isa {
>  
>  static struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
>  
> +/* Bits that can still enable any inclusion of a builtin.  */
> +static HOST_WIDE_INT deferred_isa_values = 0;
>  
>  /* Add an ix86 target builtin function with CODE, NAME and TYPE.  Save the MASK
>     of which isa_flags to use in the ix86_builtins_isa array.  Stores the
> @@ -30631,6 +30633,9 @@ def_builtin (HOST_WIDE_INT mask, const char *name,
>  	}
>        else
>  	{
> +	  /* Just a MASK where set_and_not_built_p == true can potentially
> +	     include a builtin.  */
> +	  deferred_isa_values |= mask;
>  	  ix86_builtins[(int) code] = NULL_TREE;
>  	  ix86_builtins_isa[(int) code].tcode = tcode;
>  	  ix86_builtins_isa[(int) code].name = name;
> @@ -30666,6 +30671,12 @@ def_builtin_const (HOST_WIDE_INT mask, const char *name,
>  static void
>  ix86_add_new_builtins (HOST_WIDE_INT isa)
>  {
> +  if ((isa & deferred_isa_values) == 0)
> +    return;
> +
> +  /* Bits in ISA value can be removed from potential isa values.  */
> +  deferred_isa_values &= ~isa;
> +
>    int i;
>    tree saved_current_target_pragma = current_target_pragma;
>    current_target_pragma = NULL_TREE;
> -- 
> 2.1.2
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-03-20 17:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 22:15 [PATCH] Speed-up def_builtin_const (ix86_valid_target_attribute) Martin Liška
2015-03-13  4:43 ` Jan Hubicka
2015-03-19 21:21   ` Martin Liška
2015-03-20  0:10     ` Jan Hubicka
2015-03-20 10:33       ` Martin Liška
2015-03-20 11:25         ` Jakub Jelinek
2015-03-20 14:07           ` Martin Liška
2015-03-20 17:50             ` Jan Hubicka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).