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
>From 3469163018d3c8ad2849bb9d241e12ae8723da90 Mon Sep 17 00:00:00 2001
From: mliska <mli...@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 <mli...@suse.cz>
Jakub Jelinek <ja...@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
