Richard Sandiford <rdsandif...@googlemail.com> writes:
> get_attr_enabled was showing up high in a -O0 compile of fold-const.ii.
> At the moment, extract_insn calls this function for every alternative
> on each extraction, which can be expensive for instructions like
> moves that have many alternatives.
>
> The attribute is only supposed to depend on the insn code and the
> current target. It isn't allowed to depend on the values of operands.
> LRA already makes use of this to cache the enabled attributes based on code.
>
> This patch makes the caching compiler-wide. It also converts the bool
> array into a plain int bitfield, since at the moment we don't allow more
> than 30 alternatives. (It's possible we might want to increase it
> beyond 32 at some point, but then we can change the type to uint64_t
> instead. Wanting to go beyond 64 would suggest deeper problems
> somewhere. :-))
>
> The patch gives a consistent compile-time improvement of about ~3.5%
> on the -O0 fold-const.ii testcase.
>
> There were a few instances of the construct:
>
> cl = NO_REGS;
> for (ignore_p = false, curr_alt = 0;
> (c = *constraints);
> constraints += CONSTRAINT_LEN (c, constraints))
> if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
> ignore_p = true;
> else if (c == ',')
> {
> curr_alt++;
> ignore_p = false;
> }
> else if (! ignore_p)
>
> This had the effect of ignoring all alternatives after the first
> attribute-disabled one, since once we found a disabled alternative we'd
> always enter the first arm of the "if" and never increment curr_alt.
> I don't think that was intentional.
>
> Tested on x86_64-linux-gnu. OK to install?
>
> I notice ira_setup_alts is using a HARD_REG_SET to store a mask
> of alternatives. If this patch is OK, I'll follow up with a patch
> to use alternative_mask there too.
>
> Thanks,
> Richard
>
>
> gcc/
> * system.h (TEST_BIT): New macro.
> * recog.h (alternative_mask): New type.
> (ALL_ALTERNATIVES, ALTERNATIVE_BIT): New macros.
> (recog_data_d): Replace alternative_enabled_p array with
> enabled_alternatives.
> (target_recog): New structure.
> (default_target_recog, this_target_recog): Declare.
> (get_enabled_alternatives): Likewise.
> * recog.c (default_target_recog, this_target_recog): New variables.
> (get_enabled_alternatives): New function.
> (extract_insn): Use it.
> (preprocess_constraints, constrain_operands): Adjust for change to
> recog_data.
> * postreload.c (reload_cse_simplify_operands): Likewise.
> * reload.c (find_reloads): Likewise.
> * ira-costs.c (record_reg_classes): Likewise.
> * ira-lives.c (single_reg_class): Likewise. Fix bug in which
> all alternatives after a disabled one would be skipped.
> (ira_implicitly_set_insn_hard_regs): Likewise.
> * ira.c (commutative_constraint_p): Likewise.
> (ira_setup_alts): Adjust for change to recog_data.
> * lra-int.h (lra_insn_recog_data): Replace alternative_enabled_p
> with enabled_alternatives.
> * lra.c (free_insn_recog_data): Update accordingly.
> (lra_update_insn_recog_data): Likewise.
> (lra_set_insn_recog_data): Likewise. Use get_enabled_alternatives.
> * lra-constraints.c (process_alt_operands): Likewise. Handle
> only_alternative as part of the enabled mask.
> * target-globals.h (this_target_recog): Declare.
> (target_globals): Add a recog field.
> (restore_target_globals): Restore this_target_recog.
> * target-globals.c: Include recog.h.
> (default_target_globals): Initialize recog field.
> (save_target_globals): Likewise.
Bah, EWRONGPATCH.
Index: gcc/system.h
===================================================================
--- gcc/system.h 2014-05-20 08:23:13.770215727 +0100
+++ gcc/system.h 2014-05-20 09:14:18.343945817 +0100
@@ -1070,6 +1070,9 @@ #define DEBUG_FUNCTION
#define DEBUG_VARIABLE
#endif
+/* General macro to extract bit Y of X. */
+#define TEST_BIT(X, Y) (((X) >> (Y)) & 1)
+
/* Get definitions of HOST_WIDE_INT and HOST_WIDEST_INT. */
#include "hwint.h"
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2014-05-20 08:23:13.770215727 +0100
+++ gcc/recog.h 2014-05-20 09:14:18.344945827 +0100
@@ -20,8 +20,17 @@ Software Foundation; either version 3, o
#ifndef GCC_RECOG_H
#define GCC_RECOG_H
-/* Random number that should be large enough for all purposes. */
+/* Random number that should be large enough for all purposes. Also define
+ a type that has at least MAX_RECOG_ALTERNATIVES + 1 bits, with the extra
+ bit giving an invalid value that can be used to mean "uninitialized". */
#define MAX_RECOG_ALTERNATIVES 30
+typedef unsigned int alternative_mask;
+
+/* A mask of all alternatives. */
+#define ALL_ALTERNATIVES ((alternative_mask) -1)
+
+/* A mask containing just alternative X. */
+#define ALTERNATIVE_BIT(X) ((alternative_mask) 1 << (X))
/* Types of operands. */
enum op_type {
@@ -235,11 +244,11 @@ struct recog_data_d
/* True if insn is ASM_OPERANDS. */
bool is_asm;
- /* Specifies whether an insn alternative is enabled using the
- `enabled' attribute in the insn pattern definition. For back
- ends not using the `enabled' attribute the array fields are
- always set to `true' in expand_insn. */
- bool alternative_enabled_p [MAX_RECOG_ALTERNATIVES];
+ /* Specifies whether an insn alternative is enabled using the `enabled'
+ attribute in the insn pattern definition. For back ends not using
+ the `enabled' attribute the bits are always set to 1 in expand_insn.
+ Bits beyond the last alternative are also set to 1. */
+ alternative_mask enabled_alternatives;
/* In case we are caching, hold insn data was generated for. */
rtx insn;
@@ -361,4 +370,22 @@ struct insn_data_d
extern const struct insn_data_d insn_data[];
extern int peep2_current_count;
+#ifndef GENERATOR_FILE
+#include "insn-codes.h"
+
+/* Target-dependent globals. */
+struct target_recog {
+ alternative_mask x_enabled_alternatives[LAST_INSN_CODE];
+};
+
+extern struct target_recog default_target_recog;
+#if SWITCHABLE_TARGET
+extern struct target_recog *this_target_recog;
+#else
+#define this_target_recog (&default_target_recog)
+#endif
+
+alternative_mask get_enabled_alternatives (rtx);
+#endif
+
#endif /* GCC_RECOG_H */
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/recog.c 2014-05-20 09:14:35.232103084 +0100
@@ -61,6 +61,11 @@ static void validate_replace_rtx_1 (rtx
static void validate_replace_src_1 (rtx *, void *);
static rtx split_insn (rtx);
+struct target_recog default_target_recog;
+#if SWITCHABLE_TARGET
+struct target_recog *this_target_recog = &default_target_recog;
+#endif
+
/* Nonzero means allow operands to be volatile.
This should be 0 if you are generating rtl, such as if you are calling
the functions in optabs.c and expmed.c (most of the time).
@@ -2137,6 +2142,48 @@ mode_dependent_address_p (rtx addr, addr
return targetm.mode_dependent_address_p (addr, addrspace);
}
+/* Return the mask of operand alternatives that are allowed for INSN.
+ This mask depends only on INSN and on the current target; it does not
+ depend on things like the values of operands. */
+
+alternative_mask
+get_enabled_alternatives (rtx insn)
+{
+ /* Quick exit for asms and for targets that don't use the "enabled"
+ attribute. */
+ int code = INSN_CODE (insn);
+ if (code < 0 || !HAVE_ATTR_enabled)
+ return ALL_ALTERNATIVES;
+
+ /* Calling get_attr_enabled can be expensive, so cache the mask
+ for speed. */
+ if (this_target_recog->x_enabled_alternatives[code])
+ return this_target_recog->x_enabled_alternatives[code];
+
+ /* Temporarily install enough information for get_attr_enabled to assume
+ that the insn operands are already cached. As above, the attribute
+ mustn't depend on the values of operands, so we don't provide their
+ real values here. */
+ rtx old_insn = recog_data.insn;
+ int old_alternative = which_alternative;
+
+ recog_data.insn = insn;
+ alternative_mask enabled = ALL_ALTERNATIVES;
+ int n_alternatives = insn_data[code].n_alternatives;
+ for (int i = 0; i < n_alternatives; i++)
+ {
+ which_alternative = i;
+ if (!get_attr_enabled (insn))
+ enabled &= ~ALTERNATIVE_BIT (i);
+ }
+
+ recog_data.insn = old_insn;
+ which_alternative = old_alternative;
+
+ this_target_recog->x_enabled_alternatives[code] = enabled;
+ return enabled;
+}
+
/* Like extract_insn, but save insn extracted and don't extract again, when
called again for the same insn expecting that recog_data still contain the
valid information. This is used primary by gen_attr infrastructure that
@@ -2269,19 +2316,7 @@ extract_insn (rtx insn)
gcc_assert (recog_data.n_alternatives <= MAX_RECOG_ALTERNATIVES);
- if (INSN_CODE (insn) < 0)
- for (i = 0; i < recog_data.n_alternatives; i++)
- recog_data.alternative_enabled_p[i] = true;
- else
- {
- recog_data.insn = insn;
- for (i = 0; i < recog_data.n_alternatives; i++)
- {
- which_alternative = i;
- recog_data.alternative_enabled_p[i]
- = HAVE_ATTR_enabled ? get_attr_enabled (insn) : 1;
- }
- }
+ recog_data.enabled_alternatives = get_enabled_alternatives (insn);
recog_data.insn = NULL;
which_alternative = -1;
@@ -2314,7 +2349,7 @@ preprocess_constraints (void)
op_alt[j].matches = -1;
op_alt[j].matched = -1;
- if (!recog_data.alternative_enabled_p[j])
+ if (!TEST_BIT (recog_data.enabled_alternatives, j))
{
p = skip_alternative (p);
continue;
@@ -2490,7 +2525,7 @@ constrain_operands (int strict)
int lose = 0;
funny_match_index = 0;
- if (!recog_data.alternative_enabled_p[which_alternative])
+ if (!TEST_BIT (recog_data.enabled_alternatives, which_alternative))
{
int i;
Index: gcc/postreload.c
===================================================================
--- gcc/postreload.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/postreload.c 2014-05-20 09:14:18.338945771 +0100
@@ -584,7 +584,7 @@ reload_cse_simplify_operands (rtx insn,
alternative yet and the operand being replaced is not
a cheap CONST_INT. */
if (op_alt_regno[i][j] == -1
- && recog_data.alternative_enabled_p[j]
+ && TEST_BIT (recog_data.enabled_alternatives, j)
&& reg_fits_class_p (testreg, rclass, 0, mode)
&& (!CONST_INT_P (recog_data.operand[i])
|| (set_src_cost (recog_data.operand[i],
Index: gcc/reload.c
===================================================================
--- gcc/reload.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/reload.c 2014-05-20 09:14:18.342945808 +0100
@@ -3010,7 +3010,7 @@ find_reloads (rtx insn, int replace, int
{
int swapped;
- if (!recog_data.alternative_enabled_p[this_alternative_number])
+ if (!TEST_BIT (recog_data.enabled_alternatives, this_alternative_number))
{
int i;
Index: gcc/ira-costs.c
===================================================================
--- gcc/ira-costs.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/ira-costs.c 2014-05-20 09:14:18.331945705 +0100
@@ -421,7 +421,7 @@ record_reg_classes (int n_alts, int n_op
int alt_fail = 0;
int alt_cost = 0, op_cost_add;
- if (!recog_data.alternative_enabled_p[alt])
+ if (!TEST_BIT (recog_data.enabled_alternatives, alt))
{
for (i = 0; i < recog_data.n_operands; i++)
constraints[i] = skip_alternative (constraints[i]);
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/ira-lives.c 2014-05-20 09:14:18.332945715 +0100
@@ -743,22 +743,17 @@ mark_hard_reg_early_clobbers (rtx insn,
static enum reg_class
single_reg_class (const char *constraints, rtx op, rtx equiv_const)
{
- int curr_alt, c;
- bool ignore_p;
+ int c;
enum reg_class cl, next_cl;
cl = NO_REGS;
- for (ignore_p = false, curr_alt = 0;
- (c = *constraints);
- constraints += CONSTRAINT_LEN (c, constraints))
- if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
- ignore_p = true;
+ alternative_mask enabled = recog_data.enabled_alternatives;
+ for (; (c = *constraints); constraints += CONSTRAINT_LEN (c, constraints))
+ if (c == '#')
+ enabled &= ~ALTERNATIVE_BIT (0);
else if (c == ',')
- {
- curr_alt++;
- ignore_p = false;
- }
- else if (! ignore_p)
+ enabled >>= 1;
+ else if (enabled & 1)
switch (c)
{
case ' ':
@@ -887,8 +882,7 @@ single_reg_operand_class (int op_num)
void
ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set)
{
- int i, curr_alt, c, regno = 0;
- bool ignore_p;
+ int i, c, regno = 0;
enum reg_class cl;
rtx op;
enum machine_mode mode;
@@ -909,17 +903,13 @@ ira_implicitly_set_insn_hard_regs (HARD_
mode = (GET_CODE (op) == SCRATCH
? GET_MODE (op) : PSEUDO_REGNO_MODE (regno));
cl = NO_REGS;
- for (ignore_p = false, curr_alt = 0;
- (c = *p);
- p += CONSTRAINT_LEN (c, p))
- if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
- ignore_p = true;
+ alternative_mask enabled = recog_data.enabled_alternatives;
+ for (; (c = *p); p += CONSTRAINT_LEN (c, p))
+ if (c == '#')
+ enabled &= ~ALTERNATIVE_BIT (0);
else if (c == ',')
- {
- curr_alt++;
- ignore_p = false;
- }
- else if (! ignore_p)
+ enabled >>= 1;
+ else if (enabled & 1)
switch (c)
{
case 'r':
Index: gcc/ira.c
===================================================================
--- gcc/ira.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/ira.c 2014-05-20 09:14:18.334945733 +0100
@@ -1774,23 +1774,20 @@ setup_prohibited_mode_move_regs (void)
static bool
commutative_constraint_p (const char *str)
{
- int curr_alt, c;
- bool ignore_p;
+ int c;
- for (ignore_p = false, curr_alt = 0;;)
+ alternative_mask enabled = recog_data.enabled_alternatives;
+ for (;;)
{
c = *str;
if (c == '\0')
break;
str += CONSTRAINT_LEN (c, str);
- if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
- ignore_p = true;
+ if (c == '#')
+ enabled &= ~ALTERNATIVE_BIT (0);
else if (c == ',')
- {
- curr_alt++;
- ignore_p = false;
- }
- else if (! ignore_p)
+ enabled >>= 1;
+ else if (enabled & 1)
{
/* Usually `%' is the first constraint character but the
documentation does not require this. */
@@ -1844,7 +1841,8 @@ ira_setup_alts (rtx insn, HARD_REG_SET &
}
for (nalt = 0; nalt < recog_data.n_alternatives; nalt++)
{
- if (! recog_data.alternative_enabled_p[nalt] || TEST_HARD_REG_BIT
(alts, nalt))
+ if (!TEST_BIT (recog_data.enabled_alternatives, nalt)
+ || TEST_HARD_REG_BIT (alts, nalt))
continue;
for (nop = 0; nop < recog_data.n_operands; nop++)
Index: gcc/lra-int.h
===================================================================
--- gcc/lra-int.h 2014-05-20 08:23:13.770215727 +0100
+++ gcc/lra-int.h 2014-05-20 09:14:18.337945761 +0100
@@ -227,7 +227,7 @@ struct lra_insn_recog_data
ending with a negative value. */
int *arg_hard_regs;
/* Alternative enabled for the insn. NULL for debug insns. */
- bool *alternative_enabled_p;
+ alternative_mask enabled_alternatives;
/* The following member value is always NULL for a debug insn. */
struct lra_insn_reg *regs;
};
Index: gcc/lra.c
===================================================================
--- gcc/lra.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/lra.c 2014-05-20 09:14:18.338945771 +0100
@@ -724,8 +724,6 @@ free_insn_recog_data (lra_insn_recog_dat
free (data->dup_loc);
if (data->arg_hard_regs != NULL)
free (data->arg_hard_regs);
- if (HAVE_ATTR_enabled && data->alternative_enabled_p != NULL)
- free (data->alternative_enabled_p);
if (data->icode < 0 && NONDEBUG_INSN_P (data->insn))
{
if (data->insn_static_data->operand_alternative != NULL)
@@ -1072,7 +1070,7 @@ lra_set_insn_recog_data (rtx insn)
data->insn_static_data = &debug_insn_static_data;
data->dup_loc = NULL;
data->arg_hard_regs = NULL;
- data->alternative_enabled_p = NULL;
+ data->enabled_alternatives = ALL_ALTERNATIVES;
data->operand_loc = XNEWVEC (rtx *, 1);
data->operand_loc[0] = &INSN_VAR_LOCATION_LOC (insn);
return data;
@@ -1132,7 +1130,7 @@ lra_set_insn_recog_data (rtx insn)
= (insn_static_data->operand[i].constraint[0] == '=' ? OP_OUT
: insn_static_data->operand[i].constraint[0] == '+' ? OP_INOUT
: OP_IN);
- data->alternative_enabled_p = NULL;
+ data->enabled_alternatives = ALL_ALTERNATIVES;
}
else
{
@@ -1159,27 +1157,7 @@ lra_set_insn_recog_data (rtx insn)
memcpy (locs, recog_data.dup_loc, n * sizeof (rtx *));
}
data->dup_loc = locs;
- if (HAVE_ATTR_enabled)
- {
- bool *bp;
-
- n = insn_static_data->n_alternatives;
- lra_assert (n >= 0);
- data->alternative_enabled_p = bp = XNEWVEC (bool, n);
- /* Cache the insn because we don't want to call extract_insn
- from get_attr_enabled as extract_insn modifies
- which_alternative. The attribute enabled should not depend
- on insn operands, operand modes, operand types, and operand
- constraints. It should depend on the architecture. If it
- is not true, we should rewrite this file code to use
- extract_insn instead of less expensive insn_extract. */
- recog_data.insn = insn;
- for (i = 0; i < n; i++)
- {
- which_alternative = i;
- bp[i] = get_attr_enabled (insn);
- }
- }
+ data->enabled_alternatives = get_enabled_alternatives (insn);
}
if (GET_CODE (PATTERN (insn)) == CLOBBER || GET_CODE (PATTERN (insn)) == USE)
insn_static_data->hard_regs = NULL;
@@ -1370,18 +1348,19 @@ lra_update_insn_recog_data (rtx insn)
#ifdef ENABLE_CHECKING
{
int i;
- bool *bp;
+ alternative_mask enabled;
n = insn_static_data->n_alternatives;
- bp = data->alternative_enabled_p;
- lra_assert (n >= 0 && bp != NULL);
+ enabled = data->enabled_alternatives;
+ lra_assert (n >= 0);
/* Cache the insn to prevent extract_insn call from
get_attr_enabled. */
recog_data.insn = insn;
for (i = 0; i < n; i++)
{
which_alternative = i;
- lra_assert (bp[i] == get_attr_enabled (insn));
+ lra_assert (TEST_BIT (enabled, i)
+ == (bool) get_attr_enabled (insn));
}
}
#endif
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/lra-constraints.c 2014-05-20 09:14:18.336945752 +0100
@@ -1557,19 +1557,16 @@ process_alt_operands (int only_alternati
together, the second alternatives go together, etc.
First loop over alternatives. */
+ alternative_mask enabled = curr_id->enabled_alternatives;
+ if (only_alternative >= 0)
+ enabled &= ALTERNATIVE_BIT (only_alternative);
+
for (nalt = 0; nalt < n_alternatives; nalt++)
{
/* Loop over operands for one constraint alternative. */
-#if HAVE_ATTR_enabled
- if (curr_id->alternative_enabled_p != NULL
- && ! curr_id->alternative_enabled_p[nalt])
- continue;
-#endif
-
- if (only_alternative >= 0 && nalt != only_alternative)
+ if (!TEST_BIT (enabled, nalt))
continue;
-
overall = losers = reject = reload_nregs = reload_sum = 0;
for (nop = 0; nop < n_operands; nop++)
{
Index: gcc/target-globals.h
===================================================================
--- gcc/target-globals.h 2014-05-20 08:23:13.770215727 +0100
+++ gcc/target-globals.h 2014-05-20 09:14:18.343945817 +0100
@@ -24,6 +24,7 @@ #define TARGET_GLOBALS_H 1
extern struct target_flag_state *this_target_flag_state;
extern struct target_regs *this_target_regs;
extern struct target_rtl *this_target_rtl;
+extern struct target_recog *this_target_recog;
extern struct target_hard_regs *this_target_hard_regs;
extern struct target_reload *this_target_reload;
extern struct target_expmed *this_target_expmed;
@@ -43,6 +44,7 @@ struct GTY(()) target_globals {
struct target_flag_state *GTY((skip)) flag_state;
void *GTY((atomic)) regs;
struct target_rtl *rtl;
+ void *GTY((atomic)) recog;
void *GTY((atomic)) hard_regs;
void *GTY((atomic)) reload;
void *GTY((atomic)) expmed;
@@ -70,6 +72,7 @@ restore_target_globals (struct target_gl
this_target_flag_state = g->flag_state;
this_target_regs = (struct target_regs *) g->regs;
this_target_rtl = g->rtl;
+ this_target_recog = (struct target_recog *) g->recog;
this_target_hard_regs = (struct target_hard_regs *) g->hard_regs;
this_target_reload = (struct target_reload *) g->reload;
this_target_expmed = (struct target_expmed *) g->expmed;
Index: gcc/target-globals.c
===================================================================
--- gcc/target-globals.c 2014-05-20 08:23:13.770215727 +0100
+++ gcc/target-globals.c 2014-05-20 09:14:18.342945808 +0100
@@ -43,12 +43,14 @@ Software Foundation; either version 3, o
#include "gcse.h"
#include "bb-reorder.h"
#include "lower-subreg.h"
+#include "recog.h"
#if SWITCHABLE_TARGET
struct target_globals default_target_globals = {
&default_target_flag_state,
&default_target_regs,
&default_target_rtl,
+ &default_target_recog,
&default_target_hard_regs,
&default_target_reload,
&default_target_expmed,
@@ -84,6 +86,7 @@ save_target_globals (void)
g->flag_state = &p->flag_state;
g->regs = ggc_internal_cleared_alloc (sizeof (struct target_regs));
g->rtl = ggc_cleared_alloc<target_rtl> ();
+ g->recog = ggc_internal_cleared_alloc (sizeof (struct target_recog));
g->hard_regs
= ggc_internal_cleared_alloc (sizeof (struct target_hard_regs));
g->reload = ggc_internal_cleared_alloc (sizeof (struct target_reload));
