* RFA: cache recog_op_alt by insn code
@ 2014-05-20 8:19 Richard Sandiford
2014-05-20 18:04 ` Jeff Law
0 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-20 8:19 UTC (permalink / raw)
To: gcc-patches
Following on from (and depending on) the last patch, process_constraints
also shows up high in the profile. This patch caches the recog_op_alt
information by insn code too. It also shrinks the size of the structure
from 1 pointer + 5 ints to 1 pointer + 2 ints:
- no target should have more than 65536 register classes :-)
- "reject" is based on a cost of 600 for '!', so 16 bits should be plenty
- "matched" and "matches" are operand numbers and so fit in 8 bits
Again it seems LRA already did the same thing locally. It has a routine
setup_operand_alternative that is mostly a direct cut-&-paste of
process_constraints. AFAICT the only difference is that it sets
up some parts of the main lra_static_insn_data structure too.
Since this code is creating cached data and should only be run once
per insn code or asm statement, I don't think the extra in-loop
lra_static_insn_data assignments really justify having two copies
of such a complicated function. I think it would be better for LRA
to use the generic routine and then fill in the lra_static_insn_data
fields on the result (basically just an OR of "is_address" and
"early_clobber" for each alternative, plus setting up "commutative").
We could then avoid having two caches of the same data. I'll do
that as a follow-up if it sounds OK.
On the subject of commutativity, we have:
static bool
commutative_constraint_p (const char *str)
{
int curr_alt, c;
bool ignore_p;
for (ignore_p = false, curr_alt = 0;;)
{
c = *str;
if (c == '\0')
break;
str += CONSTRAINT_LEN (c, str);
if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
ignore_p = true;
else if (c == ',')
{
curr_alt++;
ignore_p = false;
}
else if (! ignore_p)
{
/* Usually `%' is the first constraint character but the
documentation does not require this. */
if (c == '%')
return true;
}
}
return false;
}
Any objections to requiring `%' to be the first constraint character?
Seems wasteful to be searching the constraint string just to support
an odd case.
The patch gives a further ~3.5% improvement in compile time for
-O0 fold-const.ii, on top of the other patch.
Tested on x86_64-linux-gnu. OK to install?
Thanks,
Richard
gcc/
* recog.h (operand_alternative): Convert reg_class, reject,
matched and matches into bitfields.
(preprocess_constraints): Take the insn as parameter.
(recog_op_alt): Change into an array of pointers.
(target_recog): Add x_op_alt.
* recog.c (asm_op_alt_1, asm_op_alt): New variables
(recog_op_alt): Change into an array of pointers.
(preprocess_constraints): Update accordingly. Take the insn as
parameter. Use asm_op_alt_1 and asm_op_alt for asms. Cache other
instructions in this_target_recog.
* ira-lives.c (process_bb_node_lives): Pass the insn to
process_constraints.
* reg-stack.c (check_asm_stack_operands): Likewise.
(subst_asm_stack_regs): Likewise.
* regcprop.c (copyprop_hardreg_forward_1): Likewise.
* regrename.c (build_def_use): Likewise.
* sched-deps.c (sched_analyze_insn): Likewise.
* sel-sched.c (get_reg_class, implicit_clobber_conflict_p): Likewise.
* config/arm/arm.c (xscale_sched_adjust_cost): Likewise.
(note_invalid_constants): Likewise.
* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2014-05-19 20:42:50.830279171 +0100
+++ gcc/recog.h 2014-05-19 21:02:22.926604180 +0100
@@ -46,18 +46,18 @@ struct operand_alternative
const char *constraint;
/* The register class valid for this alternative (possibly NO_REGS). */
- enum reg_class cl;
+ ENUM_BITFIELD (reg_class) cl : 16;
/* "Badness" of this alternative, computed from number of '?' and '!'
characters in the constraint string. */
- unsigned int reject;
+ unsigned int reject : 16;
/* -1 if no matching constraint was found, or an operand number. */
- int matches;
+ int matches : 8;
/* The same information, but reversed: -1 if this operand is not
matched by any other, or the operand number of the operand that
matches this one. */
- int matched;
+ int matched : 8;
/* Nonzero if '&' was found in the constraint string. */
unsigned int earlyclobber:1;
@@ -77,8 +77,9 @@ struct operand_alternative
/* Nonzero if 'X' was found in the constraint string, or if the constraint
string for this alternative was empty. */
unsigned int anything_ok:1;
-};
+ unsigned int unused : 8;
+};
extern void init_recog (void);
extern void init_recog_no_volatile (void);
@@ -134,7 +135,7 @@ extern void insn_extract (rtx);
extern void extract_insn (rtx);
extern void extract_constrain_insn_cached (rtx);
extern void extract_insn_cached (rtx);
-extern void preprocess_constraints (void);
+extern void preprocess_constraints (rtx);
extern rtx peep2_next_insn (int);
extern int peep2_regno_dead_p (int, int);
extern int peep2_reg_dead_p (int, rtx);
@@ -258,7 +259,7 @@ struct recog_data_d
/* Contains a vector of operand_alternative structures for every operand.
Set up by preprocess_constraints. */
-extern struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS][MAX_RECOG_ALTERNATIVES];
+extern operand_alternative **recog_op_alt;
/* A table defined in insn-output.c that give information about
each insn-code value. */
@@ -376,6 +377,7 @@ struct insn_data_d
/* Target-dependent globals. */
struct target_recog {
alternative_mask x_enabled_alternatives[LAST_INSN_CODE];
+ operand_alternative **x_op_alt[LAST_INSN_CODE];
};
extern struct target_recog default_target_recog;
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-05-19 20:43:28.978647678 +0100
+++ gcc/recog.c 2014-05-19 23:26:02.037573014 +0100
@@ -80,7 +80,11 @@ struct recog_data_d recog_data;
/* Contains a vector of operand_alternative structures for every operand.
Set up by preprocess_constraints. */
-struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS][MAX_RECOG_ALTERNATIVES];
+operand_alternative **recog_op_alt;
+
+static operand_alternative asm_op_alt_1[MAX_RECOG_OPERANDS
+ * MAX_RECOG_ALTERNATIVES];
+static operand_alternative *asm_op_alt[MAX_RECOG_OPERANDS];
/* On return from `constrain_operands', indicate which alternative
was satisfied. */
@@ -2326,23 +2330,43 @@ extract_insn (rtx insn)
information from the constraint strings into a more usable form.
The collected data is stored in recog_op_alt. */
void
-preprocess_constraints (void)
+preprocess_constraints (rtx insn)
{
int i;
- for (i = 0; i < recog_data.n_operands; i++)
- memset (recog_op_alt[i], 0, (recog_data.n_alternatives
- * sizeof (struct operand_alternative)));
+ int code = INSN_CODE (insn);
+ if (code >= 0 && this_target_recog->x_op_alt[code])
+ {
+ recog_op_alt = this_target_recog->x_op_alt[code];
+ return;
+ }
+
+ int n_alternatives = recog_data.n_alternatives;
+ int n_operands = recog_data.n_operands;
+ int n_entries = n_operands * n_alternatives;
+
+ operand_alternative *op_alt;
+ if (code < 0)
+ {
+ memset (asm_op_alt_1, 0, n_entries * sizeof (operand_alternative));
+ op_alt = asm_op_alt_1;
+ recog_op_alt = asm_op_alt;
+ }
+ else
+ {
+ op_alt = XCNEWVEC (operand_alternative, n_entries);
+ recog_op_alt = XNEWVEC (operand_alternative *, n_operands);
+ this_target_recog->x_op_alt[code] = recog_op_alt;
+ }
- for (i = 0; i < recog_data.n_operands; i++)
+ for (i = 0; i < n_operands; i++, op_alt += n_alternatives)
{
int j;
- struct operand_alternative *op_alt;
const char *p = recog_data.constraints[i];
- op_alt = recog_op_alt[i];
+ recog_op_alt[i] = op_alt;
- for (j = 0; j < recog_data.n_alternatives; j++)
+ for (j = 0; j < n_alternatives; j++)
{
op_alt[j].cl = NO_REGS;
op_alt[j].constraint = p;
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-05-19 20:42:50.819279065 +0100
+++ gcc/ira-lives.c 2014-05-19 21:01:12.577922691 +0100
@@ -1238,7 +1238,7 @@ process_bb_node_lives (ira_loop_tree_nod
}
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
process_single_reg_class_operands (false, freq);
/* See which defined values die here. */
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c 2014-05-16 09:47:34.336936052 +0100
+++ gcc/reg-stack.c 2014-05-19 21:01:50.349288768 +0100
@@ -473,7 +473,7 @@ check_asm_stack_operands (rtx insn)
constrain_operands (1);
alt = which_alternative;
- preprocess_constraints ();
+ preprocess_constraints (insn);
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
@@ -2032,7 +2032,7 @@ subst_asm_stack_regs (rtx insn, stack_pt
constrain_operands (1);
alt = which_alternative;
- preprocess_constraints ();
+ preprocess_constraints (insn);
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c 2014-05-17 07:59:06.436021428 +0100
+++ gcc/regcprop.c 2014-05-19 21:01:29.001081988 +0100
@@ -774,7 +774,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
alt = which_alternative;
n_ops = recog_data.n_operands;
is_asm = asm_noperands (PATTERN (insn)) >= 0;
@@ -880,7 +880,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
}
/* Otherwise, try all valid registers and see if its valid. */
@@ -908,7 +908,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
}
}
}
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c 2014-05-06 18:38:47.928200023 +0100
+++ gcc/regrename.c 2014-05-19 21:01:35.724147073 +0100
@@ -1571,7 +1571,7 @@ build_def_use (basic_block bb)
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
alt = which_alternative;
n_ops = recog_data.n_operands;
untracked_operands = 0;
Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c 2014-05-16 09:47:34.347936161 +0100
+++ gcc/sched-deps.c 2014-05-19 21:01:58.526367964 +0100
@@ -2865,7 +2865,7 @@ sched_analyze_insn (struct deps_desc *de
HARD_REG_SET temp;
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
ira_implicitly_set_insn_hard_regs (&temp);
AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
IOR_HARD_REG_SET (implicit_reg_pending_clobbers, temp);
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c 2014-03-04 21:19:43.120097702 +0000
+++ gcc/sel-sched.c 2014-05-19 21:02:14.471522363 +0100
@@ -1019,7 +1019,7 @@ get_reg_class (rtx insn)
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
alt = which_alternative;
n_ops = recog_data.n_operands;
@@ -2141,7 +2141,7 @@ implicit_clobber_conflict_p (insn_t thro
/* Calculate implicit clobbers. */
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
ira_implicitly_set_insn_hard_regs (&temp);
AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c 2014-05-19 07:46:29.013179879 +0100
+++ gcc/config/arm/arm.c 2014-05-19 21:05:50.289605508 +0100
@@ -11335,7 +11335,7 @@ xscale_sched_adjust_cost (rtx insn, rtx
that overlaps with SHIFTED_OPERAND, then we have increase the
cost of this dependency. */
extract_insn (dep);
- preprocess_constraints ();
+ preprocess_constraints (dep);
for (opno = 0; opno < recog_data.n_operands; opno++)
{
/* We can ignore strict inputs. */
@@ -16870,7 +16870,7 @@ note_invalid_constants (rtx insn, HOST_W
/* Fill in recog_op_alt with information about the constraints of
this insn. */
- preprocess_constraints ();
+ preprocess_constraints (insn);
for (opno = 0; opno < recog_data.n_operands; opno++)
{
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c 2014-05-19 07:46:26.771160339 +0100
+++ gcc/config/i386/i386.c 2014-05-19 21:05:39.461501255 +0100
@@ -5826,7 +5826,7 @@ ix86_legitimate_combined_insn (rtx insn)
int i;
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
for (i = 0; i < recog_data.n_operands; i++)
{
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: RFA: cache recog_op_alt by insn code
2014-05-20 8:19 RFA: cache recog_op_alt by insn code Richard Sandiford
@ 2014-05-20 18:04 ` Jeff Law
2014-05-26 19:21 ` Require '%' to be at the beginning of a constraint string Richard Sandiford
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
0 siblings, 2 replies; 20+ messages in thread
From: Jeff Law @ 2014-05-20 18:04 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/20/14 02:19, Richard Sandiford wrote:
> Following on from (and depending on) the last patch, process_constraints
> also shows up high in the profile. This patch caches the recog_op_alt
> information by insn code too. It also shrinks the size of the structure
> from 1 pointer + 5 ints to 1 pointer + 2 ints:
>
> - no target should have more than 65536 register classes :-)
That could probably be much lower if we needed more bits for other things.
> - "reject" is based on a cost of 600 for '!', so 16 bits should be plenty
OK. Makes sense.
> - "matched" and "matches" are operand numbers and so fit in 8 bits
OK. This could also be smaller, don't we have an upper limit of 32 (or
30?) on the number of operands appearing in an insn. It'd be a way to
get a few more bits if we need them someday.
> Since this code is creating cached data and should only be run once
> per insn code or asm statement, I don't think the extra in-loop
> lra_static_insn_data assignments really justify having two copies
> of such a complicated function. I think it would be better for LRA
> to use the generic routine and then fill in the lra_static_insn_data
> fields on the result (basically just an OR of "is_address" and
> "early_clobber" for each alternative, plus setting up "commutative").
> We could then avoid having two caches of the same data. I'll do
> that as a follow-up if it sounds OK.
Seems reasonable. I suspect Vlad found the same code to be hot, hence
the caching. Once he had a copy adding to it wasn't a big deal. But
yes, I think that after the cache is globalized that having IRA walk
over things another time shouldn't be a problem.
>
> On the subject of commutativity, we have:
>
> static bool
> commutative_constraint_p (const char *str)
> {
> int curr_alt, c;
> bool ignore_p;
>
> for (ignore_p = false, curr_alt = 0;;)
> {
> c = *str;
> if (c == '\0')
> break;
> str += CONSTRAINT_LEN (c, str);
> if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
> ignore_p = true;
> else if (c == ',')
> {
> curr_alt++;
> ignore_p = false;
> }
> else if (! ignore_p)
> {
> /* Usually `%' is the first constraint character but the
> documentation does not require this. */
> if (c == '%')
> return true;
> }
> }
> return false;
> }
>
> Any objections to requiring `%' to be the first constraint character?
> Seems wasteful to be searching the constraint string just to support
> an odd case.
If we're going to change it, then clearly the docs need to change and
ideally we'd statically check the port's constraints during the build
process to ensure they meet the tighter definition.
I'm sure David will be oh-so-happy to see state appearing. But that's
something he'll have to deal with in the JIT side.
>
> The patch gives a further ~3.5% improvement in compile time for
> -O0 fold-const.ii, on top of the other patch.
>
> Tested on x86_64-linux-gnu. OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> * recog.h (operand_alternative): Convert reg_class, reject,
> matched and matches into bitfields.
> (preprocess_constraints): Take the insn as parameter.
> (recog_op_alt): Change into an array of pointers.
> (target_recog): Add x_op_alt.
> * recog.c (asm_op_alt_1, asm_op_alt): New variables
> (recog_op_alt): Change into an array of pointers.
> (preprocess_constraints): Update accordingly. Take the insn as
> parameter. Use asm_op_alt_1 and asm_op_alt for asms. Cache other
> instructions in this_target_recog.
> * ira-lives.c (process_bb_node_lives): Pass the insn to
> process_constraints.
> * reg-stack.c (check_asm_stack_operands): Likewise.
> (subst_asm_stack_regs): Likewise.
> * regcprop.c (copyprop_hardreg_forward_1): Likewise.
> * regrename.c (build_def_use): Likewise.
> * sched-deps.c (sched_analyze_insn): Likewise.
> * sel-sched.c (get_reg_class, implicit_clobber_conflict_p): Likewise.
> * config/arm/arm.c (xscale_sched_adjust_cost): Likewise.
> (note_invalid_constants): Likewise.
> * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
OK.
Thanks!
Jeff
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Require '%' to be at the beginning of a constraint string
2014-05-20 18:04 ` Jeff Law
@ 2014-05-26 19:21 ` Richard Sandiford
2014-05-27 17:41 ` Jeff Law
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
1 sibling, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-26 19:21 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Jeff Law <law@redhat.com> writes:
> On 05/20/14 02:19, Richard Sandiford wrote:
>> On the subject of commutativity, we have:
>>
>> static bool
>> commutative_constraint_p (const char *str)
>> {
>> int curr_alt, c;
>> bool ignore_p;
>>
>> for (ignore_p = false, curr_alt = 0;;)
>> {
>> c = *str;
>> if (c == '\0')
>> break;
>> str += CONSTRAINT_LEN (c, str);
>> if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
>> ignore_p = true;
>> else if (c == ',')
>> {
>> curr_alt++;
>> ignore_p = false;
>> }
>> else if (! ignore_p)
>> {
>> /* Usually `%' is the first constraint character but the
>> documentation does not require this. */
>> if (c == '%')
>> return true;
>> }
>> }
>> return false;
>> }
>>
>> Any objections to requiring `%' to be the first constraint character?
>> Seems wasteful to be searching the constraint string just to support
>> an odd case.
> If we're going to change it, then clearly the docs need to change and
> ideally we'd statically check the port's constraints during the build
> process to ensure they meet the tighter definition.
OK, how does this look? I built a cc1 for one target per config/
directory to (try to) check that there were no remaining cases.
This means that we will silently ignore '%'s that are embedded in the
middle of an asm constraint string, but in a way that's already true for
most places that check for commutativity. An error seems a bit extreme
when '%' is only a hint. If we want a warning, what should the option
be called? And should it be under -Wall, -Wextra, or on by default?
Tested on x86_64-linux-gnu. OK to install?
Thanks,
Richard
gcc/
* doc/md.texi: Document that the % constraint character must
be at the beginning of the string.
* genoutput.c (validate_insn_alternatives): Check that '=',
'+' and '%' only appear at the beginning of a constraint.
* ira.c (commutative_constraint_p): Delete.
(ira_get_dup_out_num): Expect the '%' commutativity marker to be
at the start of the string.
* config/alpha/alpha.md (*movmemdi_1, *clrmemdi_1): Remove
duplicate '='s.
* config/arm/neon.md (bicdi3_neon): Likewise.
* config/vax/vax.md (sbcdi3): Likewise.
* config/h8300/h8300.md (*cmpstz): Remove duplicate '+'.
* config/arc/arc.md (mulsi_600, mulsidi_600, umulsidi_600)
(mul64): Move '%' to beginning of constraint.
* config/arm/arm.md (*xordi3_insn): Likewise.
* config/nds32/nds32.md (add<mode>3, mulsi3, andsi3, iorsi3)
(xorsi3): Likewise.
Index: gcc/doc/md.texi
===================================================================
--- gcc/doc/md.texi 2014-05-26 19:47:27.560682653 +0100
+++ gcc/doc/md.texi 2014-05-26 20:08:23.808948838 +0100
@@ -1589,7 +1589,9 @@ See, for example, the @samp{mulsi3} insn
Declares the instruction to be commutative for this operand and the
following operand. This means that the compiler may interchange the
two operands if that is the cheapest way to make all operands fit the
-constraints.
+constraints. @samp{%} applies to all alternatives and must appear as
+the first character in the constraint.
+
@ifset INTERNALS
This is often used in patterns for addition instructions
that really have only two operands: the result must go in one of the
Index: gcc/genoutput.c
===================================================================
--- gcc/genoutput.c 2014-05-26 19:47:27.560682653 +0100
+++ gcc/genoutput.c 2014-05-26 20:08:23.675947636 +0100
@@ -781,6 +781,11 @@ validate_insn_alternatives (struct data
for (p = d->operand[start].constraint; (c = *p); p += len)
{
+ if ((c == '%' || c == '=' || c == '+')
+ && p != d->operand[start].constraint)
+ error_with_line (d->lineno,
+ "character '%c' can only be used at the"
+ " beginning of a constraint string");
#ifdef USE_MD_CONSTRAINTS
if (ISSPACE (c) || strchr (indep_constraints, c))
len = 1;
Index: gcc/ira.c
===================================================================
--- gcc/ira.c 2014-05-26 20:10:28.988080881 +0100
+++ gcc/ira.c 2014-05-26 20:10:32.773115124 +0100
@@ -1769,38 +1769,6 @@ setup_prohibited_mode_move_regs (void)
}
\f
-
-/* Return TRUE if the operand constraint STR is commutative. */
-static bool
-commutative_constraint_p (const char *str)
-{
- int curr_alt, c;
- bool ignore_p;
-
- for (ignore_p = false, curr_alt = 0;;)
- {
- c = *str;
- if (c == '\0')
- break;
- str += CONSTRAINT_LEN (c, str);
- if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
- ignore_p = true;
- else if (c == ',')
- {
- curr_alt++;
- ignore_p = false;
- }
- else if (! ignore_p)
- {
- /* Usually `%' is the first constraint character but the
- documentation does not require this. */
- if (c == '%')
- return true;
- }
- }
- return false;
-}
-
/* Setup possible alternatives in ALTS for INSN. */
void
ira_setup_alts (rtx insn, HARD_REG_SET &alts)
@@ -2101,10 +2069,9 @@ ira_get_dup_out_num (int op_num, HARD_RE
if (use_commut_op_p)
break;
use_commut_op_p = true;
- if (commutative_constraint_p (recog_data.constraints[op_num]))
+ if (recog_data.constraints[op_num][0] == '%')
str = recog_data.constraints[op_num + 1];
- else if (op_num > 0 && commutative_constraint_p (recog_data.constraints
- [op_num - 1]))
+ else if (op_num > 0 && recog_data.constraints[op_num - 1][0] == '%')
str = recog_data.constraints[op_num - 1];
else
break;
Index: gcc/config/alpha/alpha.md
===================================================================
--- gcc/config/alpha/alpha.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/alpha/alpha.md 2014-05-26 20:08:23.697947834 +0100
@@ -4764,7 +4764,7 @@ (define_expand "movmemdi"
"operands[4] = gen_rtx_SYMBOL_REF (Pmode, \"OTS$MOVE\");")
(define_insn "*movmemdi_1"
- [(set (match_operand:BLK 0 "memory_operand" "=m,=m")
+ [(set (match_operand:BLK 0 "memory_operand" "=m,m")
(match_operand:BLK 1 "memory_operand" "m,m"))
(use (match_operand:DI 2 "nonmemory_operand" "r,i"))
(use (match_operand:DI 3 "immediate_operand"))
@@ -4831,7 +4831,7 @@ (define_expand "setmemdi"
})
(define_insn "*clrmemdi_1"
- [(set (match_operand:BLK 0 "memory_operand" "=m,=m")
+ [(set (match_operand:BLK 0 "memory_operand" "=m,m")
(const_int 0))
(use (match_operand:DI 1 "nonmemory_operand" "r,i"))
(use (match_operand:DI 2 "immediate_operand"))
Index: gcc/config/arm/neon.md
===================================================================
--- gcc/config/arm/neon.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/arm/neon.md 2014-05-26 20:08:23.720948042 +0100
@@ -728,7 +728,7 @@ (define_insn "bic<mode>3_neon"
;; Compare to *anddi_notdi_di.
(define_insn "bicdi3_neon"
- [(set (match_operand:DI 0 "s_register_operand" "=w,?=&r,?&r")
+ [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r")
(and:DI (not:DI (match_operand:DI 2 "s_register_operand" "w,r,0"))
(match_operand:DI 1 "s_register_operand" "w,0,r")))]
"TARGET_NEON"
Index: gcc/config/vax/vax.md
===================================================================
--- gcc/config/vax/vax.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/vax/vax.md 2014-05-26 20:08:23.776948548 +0100
@@ -423,7 +423,7 @@ (define_expand "subdi3"
"vax_expand_addsub_di_operands (operands, MINUS); DONE;")
(define_insn "sbcdi3"
- [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,=Rr")
+ [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,Rr")
(minus:DI (match_operand:DI 1 "general_addsub_di_operand" "0,I")
(match_operand:DI 2 "general_addsub_di_operand" "nRr,Rr")))]
"TARGET_QMATH"
Index: gcc/config/h8300/h8300.md
===================================================================
--- gcc/config/h8300/h8300.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/h8300/h8300.md 2014-05-26 20:08:23.791948684 +0100
@@ -3589,7 +3589,7 @@ (define_insn "*bstzhireg"
[(set_attr "cc" "clobber")])
(define_insn_and_split "*cmpstz"
- [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,+WU")
+ [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,WU")
(const_int 1)
(match_operand:QI 1 "immediate_operand" "n,n"))
(match_operator:QI 2 "eqne_operator"
Index: gcc/config/arc/arc.md
===================================================================
--- gcc/config/arc/arc.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/arc/arc.md 2014-05-26 20:08:23.590946867 +0100
@@ -1698,7 +1698,7 @@ (define_insn "mac_600"
(define_insn "mulsi_600"
[(set (match_operand:SI 2 "mlo_operand" "")
- (mult:SI (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c")
+ (mult:SI (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c")
(match_operand:SI 1 "nonmemory_operand" "Rcq#q,cL,I,Cal")))
(clobber (match_operand:SI 3 "mhi_operand" ""))]
"TARGET_MUL64_SET"
@@ -1750,7 +1750,7 @@ (define_insn "mulsi3_600_lib"
(define_insn "mulsidi_600"
[(set (reg:DI MUL64_OUT_REG)
(mult:DI (sign_extend:DI
- (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c"))
+ (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c"))
(sign_extend:DI
; assembler issue for "I", see mulsi_600
; (match_operand:SI 1 "register_operand" "Rcq#q,cL,I,Cal"))))]
@@ -1766,7 +1766,7 @@ (define_insn "mulsidi_600"
(define_insn "umulsidi_600"
[(set (reg:DI MUL64_OUT_REG)
(mult:DI (zero_extend:DI
- (match_operand:SI 0 "register_operand" "c,c,%c"))
+ (match_operand:SI 0 "register_operand" "%c,c,c"))
(sign_extend:DI
; assembler issue for "I", see mulsi_600
; (match_operand:SI 1 "register_operand" "cL,I,Cal"))))]
@@ -4134,7 +4134,7 @@ (define_insn "swap"
;; FIXME: an intrinsic for multiply is daft. Can we remove this?
(define_insn "mul64"
- [(unspec [(match_operand:SI 0 "general_operand" "q,r,r,%r")
+ [(unspec [(match_operand:SI 0 "general_operand" "%q,r,r,r")
(match_operand:SI 1 "general_operand" "q,rL,I,Cal")]
UNSPEC_MUL64)]
"TARGET_MUL64_SET"
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/arm/arm.md 2014-05-26 20:08:23.629947220 +0100
@@ -3169,7 +3169,7 @@ (define_expand "xordi3"
(define_insn_and_split "*xordi3_insn"
[(set (match_operand:DI 0 "s_register_operand" "=w,&r,&r,&r,&r,?w")
- (xor:DI (match_operand:DI 1 "s_register_operand" "w ,%0,r ,0 ,r ,w")
+ (xor:DI (match_operand:DI 1 "s_register_operand" "%w ,0,r ,0 ,r ,w")
(match_operand:DI 2 "arm_xordi_operand" "w ,r ,r ,Dg,Dg,w")))]
"TARGET_32BIT && !TARGET_IWMMXT"
{
Index: gcc/config/nds32/nds32.md
===================================================================
--- gcc/config/nds32/nds32.md 2014-05-26 19:47:27.560682653 +0100
+++ gcc/config/nds32/nds32.md 2014-05-26 20:08:23.663947527 +0100
@@ -261,7 +261,7 @@ (define_insn "extend<mode>si2"
(define_insn "add<mode>3"
[(set (match_operand:QIHISI 0 "register_operand" "= d, l, d, l, d, l, k, l, r, r")
- (plus:QIHISI (match_operand:QIHISI 1 "register_operand" " 0, l, 0, l, %0, l, 0, k, r, r")
+ (plus:QIHISI (match_operand:QIHISI 1 "register_operand" "% 0, l, 0, l, 0, l, 0, k, r, r")
(match_operand:QIHISI 2 "nds32_rimm15s_operand" " In05, In03, Iu05, Iu03, r, l, Is10, Iu06, Is15, r")))]
""
{
@@ -382,9 +382,9 @@ (define_insn "*sub_srli"
;; Multiplication instructions.
(define_insn "mulsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r")
- (mult:SI (match_operand:SI 1 "register_operand" " %0, r")
- (match_operand:SI 2 "register_operand" " w, r")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r")
+ (mult:SI (match_operand:SI 1 "register_operand" "%0, r")
+ (match_operand:SI 2 "register_operand" " w, r")))]
""
"@
mul33\t%0, %2
@@ -489,9 +489,9 @@ (define_insn "bitc"
)
(define_insn "andsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, l, l, l, l, l, l, r, r, r, r, r")
- (and:SI (match_operand:SI 1 "register_operand" " %0, r, l, l, l, l, 0, 0, r, r, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, l, l, l, l, l, l, r, r, r, r, r")
+ (and:SI (match_operand:SI 1 "register_operand" "%0, r, l, l, l, l, 0, 0, r, r, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))]
""
{
HOST_WIDE_INT mask = INTVAL (operands[2]);
@@ -585,9 +585,9 @@ (define_insn "*and_srli"
;; For V3/V3M ISA, we have 'or33' instruction.
;; So we can identify 'or Rt3,Rt3,Ra3' case and set its length to be 2.
(define_insn "iorsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, r, r")
- (ior:SI (match_operand:SI 1 "register_operand" " %0, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, r, r")
+ (ior:SI (match_operand:SI 1 "register_operand" "%0, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))]
""
{
int one_position;
@@ -645,9 +645,9 @@ (define_insn "*or_srli"
;; For V3/V3M ISA, we have 'xor33' instruction.
;; So we can identify 'xor Rt3,Rt3,Ra3' case and set its length to be 2.
(define_insn "xorsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, r, r")
- (xor:SI (match_operand:SI 1 "register_operand" " %0, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, r, r")
+ (xor:SI (match_operand:SI 1 "register_operand" "%0, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))]
""
{
int one_position;
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Require '%' to be at the beginning of a constraint string
2014-05-26 19:21 ` Require '%' to be at the beginning of a constraint string Richard Sandiford
@ 2014-05-27 17:41 ` Jeff Law
2014-05-28 19:50 ` Richard Sandiford
0 siblings, 1 reply; 20+ messages in thread
From: Jeff Law @ 2014-05-27 17:41 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/26/14 13:21, Richard Sandiford wrote:
>> If we're going to change it, then clearly the docs need to change and
>> ideally we'd statically check the port's constraints during the build
>> process to ensure they meet the tighter definition.
>
> OK, how does this look? I built a cc1 for one target per config/
> directory to (try to) check that there were no remaining cases.
>
> This means that we will silently ignore '%'s that are embedded in the
> middle of an asm constraint string, but in a way that's already true for
> most places that check for commutativity. An error seems a bit extreme
> when '%' is only a hint. If we want a warning, what should the option
> be called? And should it be under -Wall, -Wextra, or on by default?
>
> Tested on x86_64-linux-gnu. OK to install?
OK. My initial thought on adding a warning was to weed out bad
constraints. You've already done that for the in-tree ports. I'm a lot
less inclined to do much more here to help the out-of-tree ports, so
upon further review, let's not worry about the warning unless you've
already got it ready to go :-)
>
> Thanks,
> Richard
>
>
> gcc/
> * doc/md.texi: Document that the % constraint character must
> be at the beginning of the string.
> * genoutput.c (validate_insn_alternatives): Check that '=',
> '+' and '%' only appear at the beginning of a constraint.
> * ira.c (commutative_constraint_p): Delete.
> (ira_get_dup_out_num): Expect the '%' commutativity marker to be
> at the start of the string.
> * config/alpha/alpha.md (*movmemdi_1, *clrmemdi_1): Remove
> duplicate '='s.
> * config/arm/neon.md (bicdi3_neon): Likewise.
> * config/vax/vax.md (sbcdi3): Likewise.
> * config/h8300/h8300.md (*cmpstz): Remove duplicate '+'.
> * config/arc/arc.md (mulsi_600, mulsidi_600, umulsidi_600)
> (mul64): Move '%' to beginning of constraint.
> * config/arm/arm.md (*xordi3_insn): Likewise.
> * config/nds32/nds32.md (add<mode>3, mulsi3, andsi3, iorsi3)
> (xorsi3): Likewise.
Those ARC port bits are odd. Why in the world would someone have the
commutative modifier on the last (and only the last) alternative. Strange.
Regardless, this is OK. Thanks,
Jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Require '%' to be at the beginning of a constraint string
2014-05-27 17:41 ` Jeff Law
@ 2014-05-28 19:50 ` Richard Sandiford
2014-05-30 16:24 ` Jeff Law
0 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-28 19:50 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Thanks for the review.
Jeff Law <law@redhat.com> writes:
> On 05/26/14 13:21, Richard Sandiford wrote:
>>> If we're going to change it, then clearly the docs need to change and
>>> ideally we'd statically check the port's constraints during the build
>>> process to ensure they meet the tighter definition.
>>
>> OK, how does this look? I built a cc1 for one target per config/
>> directory to (try to) check that there were no remaining cases.
>>
>> This means that we will silently ignore '%'s that are embedded in the
>> middle of an asm constraint string, but in a way that's already true for
>> most places that check for commutativity. An error seems a bit extreme
>> when '%' is only a hint. If we want a warning, what should the option
>> be called? And should it be under -Wall, -Wextra, or on by default?
>>
>> Tested on x86_64-linux-gnu. OK to install?
> OK. My initial thought on adding a warning was to weed out bad
> constraints. You've already done that for the in-tree ports. I'm a lot
> less inclined to do much more here to help the out-of-tree ports, so
> upon further review, let's not worry about the warning unless you've
> already got it ready to go :-)
Well, the new genoutput.c error should catch problems in out-of-tree ports.
It's just asms where the non-initial '%'s would be silently accepted
and have no effect.
David W suggested off-list that I add "Only input operands can use
@samp{%}." to the documention as well. That seemed like it was
obviously an improvement so I applied the patch with that change (below).
Thanks,
Richard
gcc/
* doc/md.texi: Document that the % constraint character must
be at the beginning of the string.
* genoutput.c (validate_insn_alternatives): Check that '=',
'+' and '%' only appear at the beginning of a constraint.
* ira.c (commutative_constraint_p): Delete.
(ira_get_dup_out_num): Expect the '%' commutativity marker to be
at the start of the string.
* config/alpha/alpha.md (*movmemdi_1, *clrmemdi_1): Remove
duplicate '='s.
* config/arm/neon.md (bicdi3_neon): Likewise.
* config/iq2000/iq2000.md (addsi3_internal, subsi3_internal, sgt_si)
(slt_si, sltu_si): Likewise.
* config/vax/vax.md (sbcdi3): Likewise.
* config/h8300/h8300.md (*cmpstz): Remove duplicate '+'.
* config/arc/arc.md (mulsi_600, mulsidi_600, umulsidi_600)
(mul64): Move '%' to beginning of constraint.
* config/arm/arm.md (*xordi3_insn): Likewise.
* config/nds32/nds32.md (add<mode>3, mulsi3, andsi3, iorsi3)
(xorsi3): Likewise.
Index: gcc/doc/md.texi
===================================================================
--- gcc/doc/md.texi 2014-05-27 11:23:48.676099753 +0100
+++ gcc/doc/md.texi 2014-05-27 11:23:53.289138410 +0100
@@ -1589,7 +1589,10 @@ See, for example, the @samp{mulsi3} insn
Declares the instruction to be commutative for this operand and the
following operand. This means that the compiler may interchange the
two operands if that is the cheapest way to make all operands fit the
-constraints.
+constraints. @samp{%} applies to all alternatives and must appear as
+the first character in the constraint. Only input operands can use
+@samp{%}.
+
@ifset INTERNALS
This is often used in patterns for addition instructions
that really have only two operands: the result must go in one of the
Index: gcc/genoutput.c
===================================================================
--- gcc/genoutput.c 2014-05-27 11:23:48.661099627 +0100
+++ gcc/genoutput.c 2014-05-27 11:23:53.284138368 +0100
@@ -781,6 +781,11 @@ validate_insn_alternatives (struct data
for (p = d->operand[start].constraint; (c = *p); p += len)
{
+ if ((c == '%' || c == '=' || c == '+')
+ && p != d->operand[start].constraint)
+ error_with_line (d->lineno,
+ "character '%c' can only be used at the"
+ " beginning of a constraint string", c);
#ifdef USE_MD_CONSTRAINTS
if (ISSPACE (c) || strchr (indep_constraints, c))
len = 1;
Index: gcc/ira.c
===================================================================
--- gcc/ira.c 2014-05-27 11:23:48.679099778 +0100
+++ gcc/ira.c 2014-05-27 11:24:23.746394430 +0100
@@ -1770,34 +1770,6 @@ setup_prohibited_mode_move_regs (void)
\f
-/* Return TRUE if the operand constraint STR is commutative. */
-static bool
-commutative_constraint_p (const char *str)
-{
- int c;
-
- alternative_mask enabled = recog_data.enabled_alternatives;
- for (;;)
- {
- c = *str;
- if (c == '\0')
- break;
- str += CONSTRAINT_LEN (c, str);
- if (c == '#')
- enabled &= ~ALTERNATIVE_BIT (0);
- else if (c == ',')
- enabled >>= 1;
- else if (enabled & 1)
- {
- /* Usually `%' is the first constraint character but the
- documentation does not require this. */
- if (c == '%')
- return true;
- }
- }
- return false;
-}
-
/* Setup possible alternatives in ALTS for INSN. */
void
ira_setup_alts (rtx insn, HARD_REG_SET &alts)
@@ -2099,10 +2071,9 @@ ira_get_dup_out_num (int op_num, HARD_RE
if (use_commut_op_p)
break;
use_commut_op_p = true;
- if (commutative_constraint_p (recog_data.constraints[op_num]))
+ if (recog_data.constraints[op_num][0] == '%')
str = recog_data.constraints[op_num + 1];
- else if (op_num > 0 && commutative_constraint_p (recog_data.constraints
- [op_num - 1]))
+ else if (op_num > 0 && recog_data.constraints[op_num - 1][0] == '%')
str = recog_data.constraints[op_num - 1];
else
break;
Index: gcc/config/alpha/alpha.md
===================================================================
--- gcc/config/alpha/alpha.md 2014-05-27 11:23:48.663099644 +0100
+++ gcc/config/alpha/alpha.md 2014-05-27 11:23:53.285138376 +0100
@@ -4764,7 +4764,7 @@ (define_expand "movmemdi"
"operands[4] = gen_rtx_SYMBOL_REF (Pmode, \"OTS$MOVE\");")
(define_insn "*movmemdi_1"
- [(set (match_operand:BLK 0 "memory_operand" "=m,=m")
+ [(set (match_operand:BLK 0 "memory_operand" "=m,m")
(match_operand:BLK 1 "memory_operand" "m,m"))
(use (match_operand:DI 2 "nonmemory_operand" "r,i"))
(use (match_operand:DI 3 "immediate_operand"))
@@ -4831,7 +4831,7 @@ (define_expand "setmemdi"
})
(define_insn "*clrmemdi_1"
- [(set (match_operand:BLK 0 "memory_operand" "=m,=m")
+ [(set (match_operand:BLK 0 "memory_operand" "=m,m")
(const_int 0))
(use (match_operand:DI 1 "nonmemory_operand" "r,i"))
(use (match_operand:DI 2 "immediate_operand"))
Index: gcc/config/arm/neon.md
===================================================================
--- gcc/config/arm/neon.md 2014-05-27 11:23:48.665099661 +0100
+++ gcc/config/arm/neon.md 2014-05-27 11:23:53.286138385 +0100
@@ -728,7 +728,7 @@ (define_insn "bic<mode>3_neon"
;; Compare to *anddi_notdi_di.
(define_insn "bicdi3_neon"
- [(set (match_operand:DI 0 "s_register_operand" "=w,?=&r,?&r")
+ [(set (match_operand:DI 0 "s_register_operand" "=w,?&r,?&r")
(and:DI (not:DI (match_operand:DI 2 "s_register_operand" "w,r,0"))
(match_operand:DI 1 "s_register_operand" "w,0,r")))]
"TARGET_NEON"
Index: gcc/config/iq2000/iq2000.md
===================================================================
--- gcc/config/iq2000/iq2000.md 2014-05-27 11:23:48.681099795 +0100
+++ gcc/config/iq2000/iq2000.md 2014-05-27 11:23:53.292138435 +0100
@@ -260,7 +260,7 @@ (define_expand "addsi3"
"")
(define_insn "addsi3_internal"
- [(set (match_operand:SI 0 "register_operand" "=d,=d")
+ [(set (match_operand:SI 0 "register_operand" "=d,d")
(plus:SI (match_operand:SI 1 "reg_or_0_operand" "dJ,dJ")
(match_operand:SI 2 "arith_operand" "d,I")))]
""
@@ -286,7 +286,7 @@ (define_expand "subsi3"
"")
(define_insn "subsi3_internal"
- [(set (match_operand:SI 0 "register_operand" "=d,=d")
+ [(set (match_operand:SI 0 "register_operand" "=d,d")
(minus:SI (match_operand:SI 1 "reg_or_0_operand" "dJ,dJ")
(match_operand:SI 2 "arith_operand" "d,I")))]
""
@@ -1229,7 +1229,7 @@ (define_insn "sne_si_zero"
(set_attr "mode" "SI")])
(define_insn "sgt_si"
- [(set (match_operand:SI 0 "register_operand" "=d,=d")
+ [(set (match_operand:SI 0 "register_operand" "=d,d")
(gt:SI (match_operand:SI 1 "register_operand" "d,d")
(match_operand:SI 2 "reg_or_0_operand" "d,J")))]
""
@@ -1240,7 +1240,7 @@ (define_insn "sgt_si"
(set_attr "mode" "SI,SI")])
(define_insn "slt_si"
- [(set (match_operand:SI 0 "register_operand" "=d,=d")
+ [(set (match_operand:SI 0 "register_operand" "=d,d")
(lt:SI (match_operand:SI 1 "register_operand" "d,d")
(match_operand:SI 2 "arith_operand" "d,I")))]
""
@@ -1273,7 +1273,7 @@ (define_insn "sgtu_si"
(set_attr "mode" "SI")])
(define_insn "sltu_si"
- [(set (match_operand:SI 0 "register_operand" "=d,=d")
+ [(set (match_operand:SI 0 "register_operand" "=d,d")
(ltu:SI (match_operand:SI 1 "register_operand" "d,d")
(match_operand:SI 2 "arith_operand" "d,I")))]
""
Index: gcc/config/vax/vax.md
===================================================================
--- gcc/config/vax/vax.md 2014-05-27 11:23:48.666099669 +0100
+++ gcc/config/vax/vax.md 2014-05-27 11:23:53.286138385 +0100
@@ -423,7 +423,7 @@ (define_expand "subdi3"
"vax_expand_addsub_di_operands (operands, MINUS); DONE;")
(define_insn "sbcdi3"
- [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,=Rr")
+ [(set (match_operand:DI 0 "nonimmediate_addsub_di_operand" "=Rr,Rr")
(minus:DI (match_operand:DI 1 "general_addsub_di_operand" "0,I")
(match_operand:DI 2 "general_addsub_di_operand" "nRr,Rr")))]
"TARGET_QMATH"
Index: gcc/config/h8300/h8300.md
===================================================================
--- gcc/config/h8300/h8300.md 2014-05-27 11:23:48.668099686 +0100
+++ gcc/config/h8300/h8300.md 2014-05-27 11:23:53.288138401 +0100
@@ -3589,7 +3589,7 @@ (define_insn "*bstzhireg"
[(set_attr "cc" "clobber")])
(define_insn_and_split "*cmpstz"
- [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,+WU")
+ [(set (zero_extract:QI (match_operand:QI 0 "bit_memory_operand" "+WU,WU")
(const_int 1)
(match_operand:QI 1 "immediate_operand" "n,n"))
(match_operator:QI 2 "eqne_operator"
Index: gcc/config/arc/arc.md
===================================================================
--- gcc/config/arc/arc.md 2014-05-27 11:23:48.650099535 +0100
+++ gcc/config/arc/arc.md 2014-05-27 11:23:53.280138334 +0100
@@ -1698,7 +1698,7 @@ (define_insn "mac_600"
(define_insn "mulsi_600"
[(set (match_operand:SI 2 "mlo_operand" "")
- (mult:SI (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c")
+ (mult:SI (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c")
(match_operand:SI 1 "nonmemory_operand" "Rcq#q,cL,I,Cal")))
(clobber (match_operand:SI 3 "mhi_operand" ""))]
"TARGET_MUL64_SET"
@@ -1750,7 +1750,7 @@ (define_insn "mulsi3_600_lib"
(define_insn "mulsidi_600"
[(set (reg:DI MUL64_OUT_REG)
(mult:DI (sign_extend:DI
- (match_operand:SI 0 "register_operand" "Rcq#q,c,c,%c"))
+ (match_operand:SI 0 "register_operand" "%Rcq#q,c,c,c"))
(sign_extend:DI
; assembler issue for "I", see mulsi_600
; (match_operand:SI 1 "register_operand" "Rcq#q,cL,I,Cal"))))]
@@ -1766,7 +1766,7 @@ (define_insn "mulsidi_600"
(define_insn "umulsidi_600"
[(set (reg:DI MUL64_OUT_REG)
(mult:DI (zero_extend:DI
- (match_operand:SI 0 "register_operand" "c,c,%c"))
+ (match_operand:SI 0 "register_operand" "%c,c,c"))
(sign_extend:DI
; assembler issue for "I", see mulsi_600
; (match_operand:SI 1 "register_operand" "cL,I,Cal"))))]
@@ -4134,7 +4134,7 @@ (define_insn "swap"
;; FIXME: an intrinsic for multiply is daft. Can we remove this?
(define_insn "mul64"
- [(unspec [(match_operand:SI 0 "general_operand" "q,r,r,%r")
+ [(unspec [(match_operand:SI 0 "general_operand" "%q,r,r,r")
(match_operand:SI 1 "general_operand" "q,rL,I,Cal")]
UNSPEC_MUL64)]
"TARGET_MUL64_SET"
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md 2014-05-27 11:23:48.654099569 +0100
+++ gcc/config/arm/arm.md 2014-05-27 11:23:53.282138351 +0100
@@ -3169,7 +3169,7 @@ (define_expand "xordi3"
(define_insn_and_split "*xordi3_insn"
[(set (match_operand:DI 0 "s_register_operand" "=w,&r,&r,&r,&r,?w")
- (xor:DI (match_operand:DI 1 "s_register_operand" "w ,%0,r ,0 ,r ,w")
+ (xor:DI (match_operand:DI 1 "s_register_operand" "%w ,0,r ,0 ,r ,w")
(match_operand:DI 2 "arm_xordi_operand" "w ,r ,r ,Dg,Dg,w")))]
"TARGET_32BIT && !TARGET_IWMMXT"
{
Index: gcc/config/nds32/nds32.md
===================================================================
--- gcc/config/nds32/nds32.md 2014-05-27 11:23:48.656099585 +0100
+++ gcc/config/nds32/nds32.md 2014-05-27 11:23:53.283138359 +0100
@@ -261,7 +261,7 @@ (define_insn "extend<mode>si2"
(define_insn "add<mode>3"
[(set (match_operand:QIHISI 0 "register_operand" "= d, l, d, l, d, l, k, l, r, r")
- (plus:QIHISI (match_operand:QIHISI 1 "register_operand" " 0, l, 0, l, %0, l, 0, k, r, r")
+ (plus:QIHISI (match_operand:QIHISI 1 "register_operand" "% 0, l, 0, l, 0, l, 0, k, r, r")
(match_operand:QIHISI 2 "nds32_rimm15s_operand" " In05, In03, Iu05, Iu03, r, l, Is10, Iu06, Is15, r")))]
""
{
@@ -382,9 +382,9 @@ (define_insn "*sub_srli"
;; Multiplication instructions.
(define_insn "mulsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r")
- (mult:SI (match_operand:SI 1 "register_operand" " %0, r")
- (match_operand:SI 2 "register_operand" " w, r")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r")
+ (mult:SI (match_operand:SI 1 "register_operand" "%0, r")
+ (match_operand:SI 2 "register_operand" " w, r")))]
""
"@
mul33\t%0, %2
@@ -489,9 +489,9 @@ (define_insn "bitc"
)
(define_insn "andsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, l, l, l, l, l, l, r, r, r, r, r")
- (and:SI (match_operand:SI 1 "register_operand" " %0, r, l, l, l, l, 0, 0, r, r, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, l, l, l, l, l, l, r, r, r, r, r")
+ (and:SI (match_operand:SI 1 "register_operand" "%0, r, l, l, l, l, 0, 0, r, r, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Izeb, Izeh, Ixls, Ix11, Ibms, Ifex, Izeb, Izeh, Iu15, Ii15, Ic15")))]
""
{
HOST_WIDE_INT mask = INTVAL (operands[2]);
@@ -585,9 +585,9 @@ (define_insn "*and_srli"
;; For V3/V3M ISA, we have 'or33' instruction.
;; So we can identify 'or Rt3,Rt3,Ra3' case and set its length to be 2.
(define_insn "iorsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, r, r")
- (ior:SI (match_operand:SI 1 "register_operand" " %0, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, r, r")
+ (ior:SI (match_operand:SI 1 "register_operand" "%0, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Iu15, Ie15")))]
""
{
int one_position;
@@ -645,9 +645,9 @@ (define_insn "*or_srli"
;; For V3/V3M ISA, we have 'xor33' instruction.
;; So we can identify 'xor Rt3,Rt3,Ra3' case and set its length to be 2.
(define_insn "xorsi3"
- [(set (match_operand:SI 0 "register_operand" "= w, r, r, r")
- (xor:SI (match_operand:SI 1 "register_operand" " %0, r, r, r")
- (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))]
+ [(set (match_operand:SI 0 "register_operand" "=w, r, r, r")
+ (xor:SI (match_operand:SI 1 "register_operand" "%0, r, r, r")
+ (match_operand:SI 2 "general_operand" " w, r, Iu15, It15")))]
""
{
int one_position;
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Require '%' to be at the beginning of a constraint string
2014-05-28 19:50 ` Richard Sandiford
@ 2014-05-30 16:24 ` Jeff Law
0 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2014-05-30 16:24 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/28/14 13:50, Richard Sandiford wrote:
> Thanks for the review.
>
> Jeff Law <law@redhat.com> writes:
>> On 05/26/14 13:21, Richard Sandiford wrote:
>>>> If we're going to change it, then clearly the docs need to change and
>>>> ideally we'd statically check the port's constraints during the build
>>>> process to ensure they meet the tighter definition.
>>>
>>> OK, how does this look? I built a cc1 for one target per config/
>>> directory to (try to) check that there were no remaining cases.
>>>
>>> This means that we will silently ignore '%'s that are embedded in the
>>> middle of an asm constraint string, but in a way that's already true for
>>> most places that check for commutativity. An error seems a bit extreme
>>> when '%' is only a hint. If we want a warning, what should the option
>>> be called? And should it be under -Wall, -Wextra, or on by default?
>>>
>>> Tested on x86_64-linux-gnu. OK to install?
>> OK. My initial thought on adding a warning was to weed out bad
>> constraints. You've already done that for the in-tree ports. I'm a lot
>> less inclined to do much more here to help the out-of-tree ports, so
>> upon further review, let's not worry about the warning unless you've
>> already got it ready to go :-)
>
> Well, the new genoutput.c error should catch problems in out-of-tree ports.
Right.
> It's just asms where the non-initial '%'s would be silently accepted
> and have no effect.
Right. And I believe that it's conservatively correct -- so we're OK
here as well.
>
> David W suggested off-list that I add "Only input operands can use
> @samp{%}." to the documention as well. That seemed like it was
> obviously an improvement so I applied the patch with that change (below).
Funny I was thinking about that when looking at the arc changes. I saw
the '%' on operand0 and was confused thinking it made no sense to have a
'%' for an output operand. Then I realized that 0/1 were inputs and 2
was the output.
Thanks for pushing this through.
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/5] Cache recog_op_alt by insn code, take 2
2014-05-20 18:04 ` Jeff Law
2014-05-26 19:21 ` Require '%' to be at the beginning of a constraint string Richard Sandiford
@ 2014-05-31 9:02 ` Richard Sandiford
2014-05-31 9:06 ` [PATCH 1/5] Flatten recog_op_alt and reorder entries Richard Sandiford
` (5 more replies)
1 sibling, 6 replies; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:02 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Sorry Jeff, while working on the follow-on LRA patch I came across
a couple of problems, so I need another round on this.
First of all, I mentioned doing a follow-on patch to make LRA and
recog use the same cache for operand_alternatives. I hadn't realised
until I wrote that patch that there's a significant difference between
the LRA and recog_op_alt versions of the information: recog_op_alt
groups alternatives together by operand whereas LRA groups operands
together by alternative. So in the cached flat array, recog_op_alt
uses [op * n_alternatives + alt] whereas LRA uses [alt * n_operands + nop].
The two could only share a cache if they used the same order.
I think the LRA ordering makes more sense. Quite a few places are
only interested in alternative which_alternative, so grouping operands
together by alternative gives a natural subarray. And there are places
in IRA (which uses recog_op_alt rather than the LRA information) where
the "for each alternative" loop is the outer one.
A second difference was that preprocess_constraints skips disabled
alternatives while LRA's setup_operand_alternative doesn't; LRA just
checks for disabled alternatives when walking the array instead.
That should make no difference in practice, but since the LRA approach
would also make it easier to precompute the array at build time,
I thought it would be a better way to go. It also gives a cleaner
interface: we can have a preprocess_insn_constraints function that
takes a (define_insn-based) insn and returns the operand_alternative
information without any global state.
Also, it turns out that sel-sched.c, regcprop.c and regrename.c
modify the recog_op_alt structure as a convenient way of handling
matching constraints. That obviously isn't a good idea if we want
other passes to reuse the data later. I've fixed that and made the
types "const" so that new instances shouldn't creep in.
Also, I hadn't realised that a define_insn with no constraints
at all has 0 alternatives rather than 1. Some passes nevertheless
unconditionally access the recog_op_alt information for alternative
which_alternative.
I could have fixed that by making n_alternatives always be >= 1
in the static insn table. Several places do have fast paths for
n_alternatives == 0 though, so I thought it would be better to
handle the 0 alternative case in preprocess_constraints as well.
All it needs is a MIN (..., 1).
So the patch has now grown to 5 subpatches:
1. make recog_op_alt a flat array and order it in the same way as LRA
2. fix the places that modified recog_op_alt locally
3. don't handle the enabled attribute in preprocess_constraints and make
sure all consumers check for disabled alternatives explicitly
4. the updated version of the original patch
5. get LRA to use the new cache too (the original follow-on patch)
Jeff Law <law@redhat.com> writes:
> On 05/20/14 02:19, Richard Sandiford wrote:
>> Following on from (and depending on) the last patch, process_constraints
>> also shows up high in the profile. This patch caches the recog_op_alt
>> information by insn code too. It also shrinks the size of the structure
>> from 1 pointer + 5 ints to 1 pointer + 2 ints:
>>
>> - no target should have more than 65536 register classes :-)
> That could probably be much lower if we needed more bits for other things.
>
>> - "reject" is based on a cost of 600 for '!', so 16 bits should be plenty
> OK. Makes sense.
>
>
>> - "matched" and "matches" are operand numbers and so fit in 8 bits
> OK. This could also be smaller, don't we have an upper limit of 32 (or
> 30?) on the number of operands appearing in an insn. It'd be a way to
> get a few more bits if we need them someday.
Yeah, we could probably squeeze everything into a single int if we needed
to and if we were prepared to have non-byte-sized integer fields.
Going from 2 ints to 1 int wouldn't help LP64 hosts as things stand
though, since we have a pointer at the beginning of the struct.
Boostrapped & regression-tested on x86_64-linux-gnu. Also tested by
building gcc.dg, g++.dg and gcc.c-torture for one target per config/
directory and making sure that there were no asm differences.
Thanks,
Richard
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/5] Flatten recog_op_alt and reorder entries
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
@ 2014-05-31 9:06 ` Richard Sandiford
2014-06-03 21:50 ` Jeff Law
2014-05-31 9:09 ` [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints Richard Sandiford
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:06 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
As described in the covering note, this patch converts recog_op_alt
into a flat array and orders the entries in the same way as the
LRA cache. Several places just want the information for alternative
which_alternative, so I added a convenience function for that.
Thanks,
Richard
gcc/
* recog.h (recog_op_alt): Convert to a flat array.
(which_op_alt): New function.
* recog.c (recog_op_alt): Convert to a flat array.
(preprocess_constraints): Update accordingly, grouping all
operands of the same alternative together, rather than the
other way around.
* ira-lives.c (check_and_make_def_conflict): Likewise.
(make_early_clobber_and_input_conflicts): Likewise.
* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
* reg-stack.c (check_asm_stack_operands): Use which_op_alt.
(subst_asm_stack_regs): Likewise.
* regcprop.c (copyprop_hardreg_forward_1): Likewise.
* regrename.c (hide_operands, record_out_operands): Likewise.
(build_def_use): Likewise.
* sel-sched.c (get_reg_class): Likewise.
* config/arm/arm.c (note_invalid_constants): Likewise.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2014-05-31 08:54:10.351219264 +0100
+++ gcc/recog.h 2014-05-31 08:57:21.608789337 +0100
@@ -256,9 +256,20 @@ struct recog_data_d
extern struct recog_data_d recog_data;
-/* Contains a vector of operand_alternative structures for every operand.
- Set up by preprocess_constraints. */
-extern struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS][MAX_RECOG_ALTERNATIVES];
+extern struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS
+ * MAX_RECOG_ALTERNATIVES];
+
+/* Return a pointer to an array in which index OP describes the constraints
+ on operand OP of the current instruction alternative (which_alternative).
+ Only valid after calling preprocess_constraints and constrain_operands. */
+
+inline static operand_alternative *
+which_op_alt ()
+{
+ gcc_checking_assert (IN_RANGE (which_alternative, 0,
+ recog_data.n_alternatives - 1));
+ return &recog_op_alt[which_alternative * recog_data.n_operands];
+}
/* A table defined in insn-output.c that give information about
each insn-code value. */
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/recog.c 2014-05-31 08:57:57.642085058 +0100
@@ -78,9 +78,11 @@ struct target_recog *this_target_recog =
struct recog_data_d recog_data;
-/* Contains a vector of operand_alternative structures for every operand.
+/* Contains a vector of operand_alternative structures, such that
+ operand OP of alternative A is at index A * n_operands + OP.
Set up by preprocess_constraints. */
-struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS][MAX_RECOG_ALTERNATIVES];
+struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS
+ * MAX_RECOG_ALTERNATIVES];
/* On return from `constrain_operands', indicate which alternative
was satisfied. */
@@ -2330,24 +2332,25 @@ preprocess_constraints (void)
{
int i;
- for (i = 0; i < recog_data.n_operands; i++)
- memset (recog_op_alt[i], 0, (recog_data.n_alternatives
- * sizeof (struct operand_alternative)));
+ int n_operands = recog_data.n_operands;
+ int n_alternatives = recog_data.n_alternatives;
+ int n_entries = n_operands * n_alternatives;
+ memset (recog_op_alt, 0, n_entries * sizeof (struct operand_alternative));
- for (i = 0; i < recog_data.n_operands; i++)
+ for (i = 0; i < n_operands; i++)
{
int j;
struct operand_alternative *op_alt;
const char *p = recog_data.constraints[i];
- op_alt = recog_op_alt[i];
+ op_alt = recog_op_alt;
- for (j = 0; j < recog_data.n_alternatives; j++)
+ for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
{
- op_alt[j].cl = NO_REGS;
- op_alt[j].constraint = p;
- op_alt[j].matches = -1;
- op_alt[j].matched = -1;
+ op_alt[i].cl = NO_REGS;
+ op_alt[i].constraint = p;
+ op_alt[i].matches = -1;
+ op_alt[i].matched = -1;
if (!TEST_BIT (recog_data.enabled_alternatives, j))
{
@@ -2357,7 +2360,7 @@ preprocess_constraints (void)
if (*p == '\0' || *p == ',')
{
- op_alt[j].anything_ok = 1;
+ op_alt[i].anything_ok = 1;
continue;
}
@@ -2385,77 +2388,77 @@ preprocess_constraints (void)
break;
case '?':
- op_alt[j].reject += 6;
+ op_alt[i].reject += 6;
break;
case '!':
- op_alt[j].reject += 600;
+ op_alt[i].reject += 600;
break;
case '&':
- op_alt[j].earlyclobber = 1;
+ op_alt[i].earlyclobber = 1;
break;
case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
{
char *end;
- op_alt[j].matches = strtoul (p, &end, 10);
- recog_op_alt[op_alt[j].matches][j].matched = i;
+ op_alt[i].matches = strtoul (p, &end, 10);
+ op_alt[op_alt[i].matches].matched = i;
p = end;
}
continue;
case TARGET_MEM_CONSTRAINT:
- op_alt[j].memory_ok = 1;
+ op_alt[i].memory_ok = 1;
break;
case '<':
- op_alt[j].decmem_ok = 1;
+ op_alt[i].decmem_ok = 1;
break;
case '>':
- op_alt[j].incmem_ok = 1;
+ op_alt[i].incmem_ok = 1;
break;
case 'V':
- op_alt[j].nonoffmem_ok = 1;
+ op_alt[i].nonoffmem_ok = 1;
break;
case 'o':
- op_alt[j].offmem_ok = 1;
+ op_alt[i].offmem_ok = 1;
break;
case 'X':
- op_alt[j].anything_ok = 1;
+ op_alt[i].anything_ok = 1;
break;
case 'p':
- op_alt[j].is_address = 1;
- op_alt[j].cl = reg_class_subunion[(int) op_alt[j].cl]
+ op_alt[i].is_address = 1;
+ op_alt[i].cl = reg_class_subunion[(int) op_alt[i].cl]
[(int) base_reg_class (VOIDmode, ADDR_SPACE_GENERIC,
ADDRESS, SCRATCH)];
break;
case 'g':
case 'r':
- op_alt[j].cl =
- reg_class_subunion[(int) op_alt[j].cl][(int) GENERAL_REGS];
+ op_alt[i].cl =
+ reg_class_subunion[(int) op_alt[i].cl][(int) GENERAL_REGS];
break;
default:
if (EXTRA_MEMORY_CONSTRAINT (c, p))
{
- op_alt[j].memory_ok = 1;
+ op_alt[i].memory_ok = 1;
break;
}
if (EXTRA_ADDRESS_CONSTRAINT (c, p))
{
- op_alt[j].is_address = 1;
- op_alt[j].cl
+ op_alt[i].is_address = 1;
+ op_alt[i].cl
= (reg_class_subunion
- [(int) op_alt[j].cl]
+ [(int) op_alt[i].cl]
[(int) base_reg_class (VOIDmode, ADDR_SPACE_GENERIC,
ADDRESS, SCRATCH)]);
break;
}
- op_alt[j].cl
+ op_alt[i].cl
= (reg_class_subunion
- [(int) op_alt[j].cl]
+ [(int) op_alt[i].cl]
[(int) REG_CLASS_FROM_CONSTRAINT ((unsigned char) c, p)]);
break;
}
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/ira-lives.c 2014-05-31 08:54:12.238234755 +0100
@@ -624,30 +624,35 @@ check_and_make_def_conflict (int alt, in
advance_p = true;
- for (use = 0; use < recog_data.n_operands; use++)
+ int n_operands = recog_data.n_operands;
+ operand_alternative *op_alt = &recog_op_alt[alt * n_operands];
+ for (use = 0; use < n_operands; use++)
{
int alt1;
if (use == def || recog_data.operand_type[use] == OP_OUT)
continue;
- if (recog_op_alt[use][alt].anything_ok)
+ if (op_alt[use].anything_ok)
use_cl = ALL_REGS;
else
- use_cl = recog_op_alt[use][alt].cl;
+ use_cl = op_alt[use].cl;
/* If there's any alternative that allows USE to match DEF, do not
record a conflict. If that causes us to create an invalid
instruction due to the earlyclobber, reload must fix it up. */
for (alt1 = 0; alt1 < recog_data.n_alternatives; alt1++)
- if (recog_op_alt[use][alt1].matches == def
- || (use < recog_data.n_operands - 1
- && recog_data.constraints[use][0] == '%'
- && recog_op_alt[use + 1][alt1].matches == def)
- || (use >= 1
- && recog_data.constraints[use - 1][0] == '%'
- && recog_op_alt[use - 1][alt1].matches == def))
- break;
+ {
+ operand_alternative *op_alt1 = &recog_op_alt[alt1 * n_operands];
+ if (op_alt1[use].matches == def
+ || (use < n_operands - 1
+ && recog_data.constraints[use][0] == '%'
+ && op_alt1[use + 1].matches == def)
+ || (use >= 1
+ && recog_data.constraints[use - 1][0] == '%'
+ && op_alt1[use - 1].matches == def))
+ break;
+ }
if (alt1 < recog_data.n_alternatives)
continue;
@@ -655,15 +660,15 @@ check_and_make_def_conflict (int alt, in
advance_p = check_and_make_def_use_conflict (dreg, orig_dreg, def_cl,
use, use_cl, advance_p);
- if ((use_match = recog_op_alt[use][alt].matches) >= 0)
+ if ((use_match = op_alt[use].matches) >= 0)
{
if (use_match == def)
continue;
- if (recog_op_alt[use_match][alt].anything_ok)
+ if (op_alt[use_match].anything_ok)
use_cl = ALL_REGS;
else
- use_cl = recog_op_alt[use_match][alt].cl;
+ use_cl = op_alt[use_match].cl;
advance_p = check_and_make_def_use_conflict (dreg, orig_dreg, def_cl,
use, use_cl, advance_p);
}
@@ -681,26 +686,29 @@ make_early_clobber_and_input_conflicts (
int def, def_match;
enum reg_class def_cl;
- for (alt = 0; alt < recog_data.n_alternatives; alt++)
- for (def = 0; def < recog_data.n_operands; def++)
+ int n_alternatives = recog_data.n_alternatives;
+ int n_operands = recog_data.n_operands;
+ operand_alternative *op_alt = recog_op_alt;
+ for (alt = 0; alt < n_alternatives; alt++, op_alt += n_operands)
+ for (def = 0; def < n_operands; def++)
{
def_cl = NO_REGS;
- if (recog_op_alt[def][alt].earlyclobber)
+ if (op_alt[def].earlyclobber)
{
- if (recog_op_alt[def][alt].anything_ok)
+ if (op_alt[def].anything_ok)
def_cl = ALL_REGS;
else
- def_cl = recog_op_alt[def][alt].cl;
+ def_cl = op_alt[def].cl;
check_and_make_def_conflict (alt, def, def_cl);
}
- if ((def_match = recog_op_alt[def][alt].matches) >= 0
- && (recog_op_alt[def_match][alt].earlyclobber
- || recog_op_alt[def][alt].earlyclobber))
+ if ((def_match = op_alt[def].matches) >= 0
+ && (op_alt[def_match].earlyclobber
+ || op_alt[def].earlyclobber))
{
- if (recog_op_alt[def_match][alt].anything_ok)
+ if (op_alt[def_match].anything_ok)
def_cl = ALL_REGS;
else
- def_cl = recog_op_alt[def_match][alt].cl;
+ def_cl = op_alt[def_match].cl;
check_and_make_def_conflict (alt, def, def_cl);
}
}
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/config/i386/i386.c 2014-05-31 08:54:12.236234739 +0100
@@ -5828,11 +5828,13 @@ ix86_legitimate_combined_insn (rtx insn)
extract_insn (insn);
preprocess_constraints ();
- for (i = 0; i < recog_data.n_operands; i++)
+ int n_operands = recog_data.n_operands;
+ int n_alternatives = recog_data.n_alternatives;
+ for (i = 0; i < n_operands; i++)
{
rtx op = recog_data.operand[i];
enum machine_mode mode = GET_MODE (op);
- struct operand_alternative *op_alt;
+ operand_alternative *op_alt;
int offset = 0;
bool win;
int j;
@@ -5867,19 +5869,19 @@ ix86_legitimate_combined_insn (rtx insn)
if (!(REG_P (op) && HARD_REGISTER_P (op)))
continue;
- op_alt = recog_op_alt[i];
+ op_alt = recog_op_alt;
/* Operand has no constraints, anything is OK. */
- win = !recog_data.n_alternatives;
+ win = !n_alternatives;
- for (j = 0; j < recog_data.n_alternatives; j++)
+ for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
{
- if (op_alt[j].anything_ok
- || (op_alt[j].matches != -1
+ if (op_alt[i].anything_ok
+ || (op_alt[i].matches != -1
&& operands_match_p
(recog_data.operand[i],
- recog_data.operand[op_alt[j].matches]))
- || reg_fits_class_p (op, op_alt[j].cl, offset, mode))
+ recog_data.operand[op_alt[i].matches]))
+ || reg_fits_class_p (op, op_alt[i].cl, offset, mode))
{
win = true;
break;
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/reg-stack.c 2014-05-31 08:57:21.608789337 +0100
@@ -462,7 +462,6 @@ check_asm_stack_operands (rtx insn)
char reg_used_as_output[FIRST_PSEUDO_REGISTER];
char implicitly_dies[FIRST_PSEUDO_REGISTER];
- int alt;
rtx *clobber_reg = 0;
int n_inputs, n_outputs;
@@ -471,19 +470,19 @@ check_asm_stack_operands (rtx insn)
alternative matches, this asm is malformed. */
extract_insn (insn);
constrain_operands (1);
- alt = which_alternative;
preprocess_constraints ();
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
- if (alt < 0)
+ if (which_alternative < 0)
{
malformed_asm = 1;
/* Avoid further trouble with this insn. */
PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
return 0;
}
+ operand_alternative *op_alt = which_op_alt ();
/* Strip SUBREGs here to make the following code simpler. */
for (i = 0; i < recog_data.n_operands; i++)
@@ -527,7 +526,7 @@ check_asm_stack_operands (rtx insn)
for (i = 0; i < n_outputs; i++)
if (STACK_REG_P (recog_data.operand[i]))
{
- if (reg_class_size[(int) recog_op_alt[i][alt].cl] != 1)
+ if (reg_class_size[(int) op_alt[i].cl] != 1)
{
error_for_asm (insn, "output constraint %d must specify a single register", i);
malformed_asm = 1;
@@ -582,7 +581,7 @@ check_asm_stack_operands (rtx insn)
if (operands_match_p (clobber_reg[j], recog_data.operand[i]))
break;
- if (j < n_clobbers || recog_op_alt[i][alt].matches >= 0)
+ if (j < n_clobbers || op_alt[i].matches >= 0)
implicitly_dies[REGNO (recog_data.operand[i])] = 1;
}
@@ -610,7 +609,7 @@ check_asm_stack_operands (rtx insn)
record any earlyclobber. */
for (i = n_outputs; i < n_outputs + n_inputs; i++)
- if (recog_op_alt[i][alt].matches == -1)
+ if (op_alt[i].matches == -1)
{
int j;
@@ -2006,7 +2005,6 @@ subst_stack_regs_pat (rtx insn, stack_pt
subst_asm_stack_regs (rtx insn, stack_ptr regstack)
{
rtx body = PATTERN (insn);
- int alt;
rtx *note_reg; /* Array of note contents */
rtx **note_loc; /* Address of REG field of each note */
@@ -2030,14 +2028,12 @@ subst_asm_stack_regs (rtx insn, stack_pt
such an insn in check_asm_stack_operands. */
extract_insn (insn);
constrain_operands (1);
- alt = which_alternative;
preprocess_constraints ();
+ operand_alternative *op_alt = which_op_alt ();
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
- gcc_assert (alt >= 0);
-
/* Strip SUBREGs here to make the following code simpler. */
for (i = 0; i < recog_data.n_operands; i++)
if (GET_CODE (recog_data.operand[i]) == SUBREG
@@ -2118,9 +2114,8 @@ subst_asm_stack_regs (rtx insn, stack_pt
for (i = n_outputs; i < n_outputs + n_inputs; i++)
if (STACK_REG_P (recog_data.operand[i])
- && reg_class_subset_p (recog_op_alt[i][alt].cl,
- FLOAT_REGS)
- && recog_op_alt[i][alt].cl != FLOAT_REGS)
+ && reg_class_subset_p (op_alt[i].cl, FLOAT_REGS)
+ && op_alt[i].cl != FLOAT_REGS)
{
/* If an operand needs to be in a particular reg in
FLOAT_REGS, the constraint was either 't' or 'u'. Since
@@ -2208,7 +2203,7 @@ subst_asm_stack_regs (rtx insn, stack_pt
if (operands_match_p (clobber_reg[j], recog_data.operand[i]))
break;
- if (j < n_clobbers || recog_op_alt[i][alt].matches >= 0)
+ if (j < n_clobbers || op_alt[i].matches >= 0)
{
/* recog_data.operand[i] might not be at the top of stack.
But that's OK, because all we need to do is pop the
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/regcprop.c 2014-05-31 08:57:21.608789337 +0100
@@ -745,7 +745,7 @@ copyprop_hardreg_forward_1 (basic_block
for (insn = BB_HEAD (bb); ; insn = NEXT_INSN (insn))
{
- int n_ops, i, alt, predicated;
+ int n_ops, i, predicated;
bool is_asm, any_replacements;
rtx set;
rtx link;
@@ -775,7 +775,7 @@ copyprop_hardreg_forward_1 (basic_block
if (! constrain_operands (1))
fatal_insn_not_found (insn);
preprocess_constraints ();
- alt = which_alternative;
+ operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
is_asm = asm_noperands (PATTERN (insn)) >= 0;
@@ -786,10 +786,10 @@ copyprop_hardreg_forward_1 (basic_block
predicated = GET_CODE (PATTERN (insn)) == COND_EXEC;
for (i = 0; i < n_ops; ++i)
{
- int matches = recog_op_alt[i][alt].matches;
+ int matches = op_alt[i].matches;
if (matches >= 0)
- recog_op_alt[i][alt].cl = recog_op_alt[matches][alt].cl;
- if (matches >= 0 || recog_op_alt[i][alt].matched >= 0
+ op_alt[i].cl = op_alt[matches].cl;
+ if (matches >= 0 || op_alt[i].matched >= 0
|| (predicated && recog_data.operand_type[i] == OP_OUT))
recog_data.operand_type[i] = OP_INOUT;
}
@@ -800,7 +800,7 @@ copyprop_hardreg_forward_1 (basic_block
/* For each earlyclobber operand, zap the value data. */
for (i = 0; i < n_ops; i++)
- if (recog_op_alt[i][alt].earlyclobber)
+ if (op_alt[i].earlyclobber)
kill_value (recog_data.operand[i], vd);
/* Within asms, a clobber cannot overlap inputs or outputs.
@@ -814,7 +814,7 @@ copyprop_hardreg_forward_1 (basic_block
/* Kill all early-clobbered operands. */
for (i = 0; i < n_ops; i++)
- if (recog_op_alt[i][alt].earlyclobber)
+ if (op_alt[i].earlyclobber)
kill_value (recog_data.operand[i], vd);
/* If we have dead sets in the insn, then we need to note these as we
@@ -936,17 +936,15 @@ copyprop_hardreg_forward_1 (basic_block
if (recog_data.operand_type[i] == OP_IN)
{
- if (recog_op_alt[i][alt].is_address)
+ if (op_alt[i].is_address)
replaced[i]
= replace_oldest_value_addr (recog_data.operand_loc[i],
- recog_op_alt[i][alt].cl,
- VOIDmode, ADDR_SPACE_GENERIC,
- insn, vd);
+ op_alt[i].cl, VOIDmode,
+ ADDR_SPACE_GENERIC, insn, vd);
else if (REG_P (recog_data.operand[i]))
replaced[i]
= replace_oldest_value_reg (recog_data.operand_loc[i],
- recog_op_alt[i][alt].cl,
- insn, vd);
+ op_alt[i].cl, insn, vd);
else if (MEM_P (recog_data.operand[i]))
replaced[i] = replace_oldest_value_mem (recog_data.operand[i],
insn, vd);
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/regrename.c 2014-05-31 08:57:21.608789337 +0100
@@ -1427,7 +1427,7 @@ hide_operands (int n_ops, rtx *old_opera
unsigned HOST_WIDE_INT do_not_hide, bool inout_and_ec_only)
{
int i;
- int alt = which_alternative;
+ operand_alternative *op_alt = which_op_alt ();
for (i = 0; i < n_ops; i++)
{
old_operands[i] = recog_data.operand[i];
@@ -1439,7 +1439,7 @@ hide_operands (int n_ops, rtx *old_opera
if (do_not_hide & (1 << i))
continue;
if (!inout_and_ec_only || recog_data.operand_type[i] == OP_INOUT
- || recog_op_alt[i][alt].earlyclobber)
+ || op_alt[i].earlyclobber)
*recog_data.operand_loc[i] = cc0_rtx;
}
for (i = 0; i < recog_data.n_dups; i++)
@@ -1449,7 +1449,7 @@ hide_operands (int n_ops, rtx *old_opera
if (do_not_hide & (1 << opn))
continue;
if (!inout_and_ec_only || recog_data.operand_type[opn] == OP_INOUT
- || recog_op_alt[opn][alt].earlyclobber)
+ || op_alt[opn].earlyclobber)
*recog_data.dup_loc[i] = cc0_rtx;
}
}
@@ -1478,7 +1478,7 @@ restore_operands (rtx insn, int n_ops, r
record_out_operands (rtx insn, bool earlyclobber, insn_rr_info *insn_info)
{
int n_ops = recog_data.n_operands;
- int alt = which_alternative;
+ operand_alternative *op_alt = which_op_alt ();
int i;
@@ -1489,12 +1489,12 @@ record_out_operands (rtx insn, bool earl
? recog_data.operand_loc[opn]
: recog_data.dup_loc[i - n_ops]);
rtx op = *loc;
- enum reg_class cl = recog_op_alt[opn][alt].cl;
+ enum reg_class cl = op_alt[opn].cl;
struct du_head *prev_open;
if (recog_data.operand_type[opn] != OP_OUT
- || recog_op_alt[opn][alt].earlyclobber != earlyclobber)
+ || op_alt[opn].earlyclobber != earlyclobber)
continue;
if (insn_info)
@@ -1539,7 +1539,6 @@ build_def_use (basic_block bb)
rtx old_operands[MAX_RECOG_OPERANDS];
rtx old_dups[MAX_DUP_OPERANDS];
int i;
- int alt;
int predicated;
enum rtx_code set_code = SET;
enum rtx_code clobber_code = CLOBBER;
@@ -1572,7 +1571,7 @@ build_def_use (basic_block bb)
if (! constrain_operands (1))
fatal_insn_not_found (insn);
preprocess_constraints ();
- alt = which_alternative;
+ operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
untracked_operands = 0;
@@ -1595,10 +1594,10 @@ build_def_use (basic_block bb)
for (i = 0; i < n_ops; ++i)
{
rtx op = recog_data.operand[i];
- int matches = recog_op_alt[i][alt].matches;
+ int matches = op_alt[i].matches;
if (matches >= 0)
- recog_op_alt[i][alt].cl = recog_op_alt[matches][alt].cl;
- if (matches >= 0 || recog_op_alt[i][alt].matched >= 0
+ op_alt[i].cl = op_alt[matches].cl;
+ if (matches >= 0 || op_alt[i].matched >= 0
|| (predicated && recog_data.operand_type[i] == OP_OUT))
{
recog_data.operand_type[i] = OP_INOUT;
@@ -1682,7 +1681,7 @@ build_def_use (basic_block bb)
rtx *loc = (i < n_ops
? recog_data.operand_loc[opn]
: recog_data.dup_loc[i - n_ops]);
- enum reg_class cl = recog_op_alt[opn][alt].cl;
+ enum reg_class cl = op_alt[opn].cl;
enum op_type type = recog_data.operand_type[opn];
/* Don't scan match_operand here, since we've no reg class
@@ -1694,7 +1693,7 @@ build_def_use (basic_block bb)
if (insn_info)
cur_operand = i == opn ? insn_info->op_info + i : NULL;
- if (recog_op_alt[opn][alt].is_address)
+ if (op_alt[opn].is_address)
scan_rtx_address (insn, loc, cl, mark_read,
VOIDmode, ADDR_SPACE_GENERIC);
else
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/sel-sched.c 2014-05-31 08:57:21.608789337 +0100
@@ -1014,20 +1014,20 @@ vinsn_writes_one_of_regs_p (vinsn_t vi,
static enum reg_class
get_reg_class (rtx insn)
{
- int alt, i, n_ops;
+ int i, n_ops;
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
preprocess_constraints ();
- alt = which_alternative;
n_ops = recog_data.n_operands;
+ operand_alternative *op_alt = which_op_alt ();
for (i = 0; i < n_ops; ++i)
{
- int matches = recog_op_alt[i][alt].matches;
+ int matches = op_alt[i].matches;
if (matches >= 0)
- recog_op_alt[i][alt].cl = recog_op_alt[matches][alt].cl;
+ op_alt[i].cl = op_alt[matches].cl;
}
if (asm_noperands (PATTERN (insn)) > 0)
@@ -1037,7 +1037,7 @@ get_reg_class (rtx insn)
{
rtx *loc = recog_data.operand_loc[i];
rtx op = *loc;
- enum reg_class cl = recog_op_alt[i][alt].cl;
+ enum reg_class cl = op_alt[i].cl;
if (REG_P (op)
&& REGNO (op) == ORIGINAL_REGNO (op))
@@ -1051,7 +1051,7 @@ get_reg_class (rtx insn)
for (i = 0; i < n_ops + recog_data.n_dups; i++)
{
int opn = i < n_ops ? i : recog_data.dup_num[i - n_ops];
- enum reg_class cl = recog_op_alt[opn][alt].cl;
+ enum reg_class cl = op_alt[opn].cl;
if (recog_data.operand_type[opn] == OP_OUT ||
recog_data.operand_type[opn] == OP_INOUT)
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c 2014-05-31 08:54:10.351219264 +0100
+++ gcc/config/arm/arm.c 2014-05-31 08:57:21.608789337 +0100
@@ -16872,6 +16872,7 @@ note_invalid_constants (rtx insn, HOST_W
this insn. */
preprocess_constraints ();
+ operand_alternative *op_alt = which_op_alt ();
for (opno = 0; opno < recog_data.n_operands; opno++)
{
/* Things we need to fix can only occur in inputs. */
@@ -16882,7 +16883,7 @@ note_invalid_constants (rtx insn, HOST_W
of constants in this alternative is really to fool reload
into allowing us to accept one there. We need to fix them up
now so that we output the right code. */
- if (recog_op_alt[opno][which_alternative].memory_ok)
+ if (op_alt[opno].memory_ok)
{
rtx op = recog_data.operand[opno];
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
2014-05-31 9:06 ` [PATCH 1/5] Flatten recog_op_alt and reorder entries Richard Sandiford
@ 2014-05-31 9:09 ` Richard Sandiford
2014-06-03 21:53 ` Jeff Law
2014-05-31 9:15 ` [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute Richard Sandiford
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:09 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Since the aim of this series is to cache the result of preprocess_constraints,
we need to make sure that passes don't modify the information afterwards.
This patch deals with the places that did. Patch 4 will make the information
properly const.
Thanks,
Richard
gcc/
* recog.h (alternative_class): New function.
(which_op_alt): Return a const recog_op_alt.
* reg-stack.c (check_asm_stack_operands): Update type accordingly.
(subst_asm_stack_regs): Likewise.
* config/arm/arm.c (note_invalid_constants): Likewise.
* regcprop.c (copyprop_hardreg_forward_1): Likewise. Don't modify
the operand_alternative; use alternative class instead.
* sel-sched.c (get_reg_class): Likewise.
* regrename.c (build_def_use): Likewise.
(hide_operands, restore_operands, record_out_operands): Update type
accordingly.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2014-05-31 08:57:21.608789337 +0100
+++ gcc/recog.h 2014-05-31 09:02:35.956369716 +0100
@@ -79,6 +79,14 @@ struct operand_alternative
unsigned int anything_ok:1;
};
+/* Return the class for operand I of alternative ALT, taking matching
+ constraints into account. */
+
+static inline enum reg_class
+alternative_class (const operand_alternative *alt, int i)
+{
+ return alt[i].matches >= 0 ? alt[alt[i].matches].cl : alt[i].cl;
+}
extern void init_recog (void);
extern void init_recog_no_volatile (void);
@@ -263,7 +271,7 @@ struct recog_data_d
on operand OP of the current instruction alternative (which_alternative).
Only valid after calling preprocess_constraints and constrain_operands. */
-inline static operand_alternative *
+inline static const operand_alternative *
which_op_alt ()
{
gcc_checking_assert (IN_RANGE (which_alternative, 0,
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c 2014-05-31 08:57:21.608789337 +0100
+++ gcc/reg-stack.c 2014-05-31 09:02:35.968369814 +0100
@@ -482,7 +482,7 @@ check_asm_stack_operands (rtx insn)
PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
return 0;
}
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
/* Strip SUBREGs here to make the following code simpler. */
for (i = 0; i < recog_data.n_operands; i++)
@@ -2030,7 +2030,7 @@ subst_asm_stack_regs (rtx insn, stack_pt
constrain_operands (1);
preprocess_constraints ();
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c 2014-05-31 08:57:21.608789337 +0100
+++ gcc/config/arm/arm.c 2014-05-31 09:02:35.966369798 +0100
@@ -16872,7 +16872,7 @@ note_invalid_constants (rtx insn, HOST_W
this insn. */
preprocess_constraints ();
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
for (opno = 0; opno < recog_data.n_operands; opno++)
{
/* Things we need to fix can only occur in inputs. */
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c 2014-05-31 08:57:21.608789337 +0100
+++ gcc/regcprop.c 2014-05-31 09:02:35.957369724 +0100
@@ -775,20 +775,17 @@ copyprop_hardreg_forward_1 (basic_block
if (! constrain_operands (1))
fatal_insn_not_found (insn);
preprocess_constraints ();
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
is_asm = asm_noperands (PATTERN (insn)) >= 0;
- /* Simplify the code below by rewriting things to reflect
- matching constraints. Also promote OP_OUT to OP_INOUT
+ /* Simplify the code below by promoting OP_OUT to OP_INOUT
in predicated instructions. */
predicated = GET_CODE (PATTERN (insn)) == COND_EXEC;
for (i = 0; i < n_ops; ++i)
{
int matches = op_alt[i].matches;
- if (matches >= 0)
- op_alt[i].cl = op_alt[matches].cl;
if (matches >= 0 || op_alt[i].matched >= 0
|| (predicated && recog_data.operand_type[i] == OP_OUT))
recog_data.operand_type[i] = OP_INOUT;
@@ -939,12 +936,14 @@ copyprop_hardreg_forward_1 (basic_block
if (op_alt[i].is_address)
replaced[i]
= replace_oldest_value_addr (recog_data.operand_loc[i],
- op_alt[i].cl, VOIDmode,
- ADDR_SPACE_GENERIC, insn, vd);
+ alternative_class (op_alt, i),
+ VOIDmode, ADDR_SPACE_GENERIC,
+ insn, vd);
else if (REG_P (recog_data.operand[i]))
replaced[i]
= replace_oldest_value_reg (recog_data.operand_loc[i],
- op_alt[i].cl, insn, vd);
+ alternative_class (op_alt, i),
+ insn, vd);
else if (MEM_P (recog_data.operand[i]))
replaced[i] = replace_oldest_value_mem (recog_data.operand[i],
insn, vd);
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c 2014-05-31 08:57:21.608789337 +0100
+++ gcc/sel-sched.c 2014-05-31 09:02:35.970369830 +0100
@@ -1022,14 +1022,7 @@ get_reg_class (rtx insn)
preprocess_constraints ();
n_ops = recog_data.n_operands;
- operand_alternative *op_alt = which_op_alt ();
- for (i = 0; i < n_ops; ++i)
- {
- int matches = op_alt[i].matches;
- if (matches >= 0)
- op_alt[i].cl = op_alt[matches].cl;
- }
-
+ const operand_alternative *op_alt = which_op_alt ();
if (asm_noperands (PATTERN (insn)) > 0)
{
for (i = 0; i < n_ops; i++)
@@ -1037,7 +1030,7 @@ get_reg_class (rtx insn)
{
rtx *loc = recog_data.operand_loc[i];
rtx op = *loc;
- enum reg_class cl = op_alt[i].cl;
+ enum reg_class cl = alternative_class (op_alt, i);
if (REG_P (op)
&& REGNO (op) == ORIGINAL_REGNO (op))
@@ -1051,7 +1044,7 @@ get_reg_class (rtx insn)
for (i = 0; i < n_ops + recog_data.n_dups; i++)
{
int opn = i < n_ops ? i : recog_data.dup_num[i - n_ops];
- enum reg_class cl = op_alt[opn].cl;
+ enum reg_class cl = alternative_class (op_alt, opn);
if (recog_data.operand_type[opn] == OP_OUT ||
recog_data.operand_type[opn] == OP_INOUT)
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c 2014-05-31 08:57:21.608789337 +0100
+++ gcc/regrename.c 2014-05-31 09:02:35.958369732 +0100
@@ -1427,7 +1427,7 @@ hide_operands (int n_ops, rtx *old_opera
unsigned HOST_WIDE_INT do_not_hide, bool inout_and_ec_only)
{
int i;
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
for (i = 0; i < n_ops; i++)
{
old_operands[i] = recog_data.operand[i];
@@ -1478,7 +1478,7 @@ restore_operands (rtx insn, int n_ops, r
record_out_operands (rtx insn, bool earlyclobber, insn_rr_info *insn_info)
{
int n_ops = recog_data.n_operands;
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
int i;
@@ -1489,7 +1489,7 @@ record_out_operands (rtx insn, bool earl
? recog_data.operand_loc[opn]
: recog_data.dup_loc[i - n_ops]);
rtx op = *loc;
- enum reg_class cl = op_alt[opn].cl;
+ enum reg_class cl = alternative_class (op_alt, opn);
struct du_head *prev_open;
@@ -1571,7 +1571,7 @@ build_def_use (basic_block bb)
if (! constrain_operands (1))
fatal_insn_not_found (insn);
preprocess_constraints ();
- operand_alternative *op_alt = which_op_alt ();
+ const operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
untracked_operands = 0;
@@ -1584,8 +1584,7 @@ build_def_use (basic_block bb)
sizeof (operand_rr_info) * recog_data.n_operands);
}
- /* Simplify the code below by rewriting things to reflect
- matching constraints. Also promote OP_OUT to OP_INOUT in
+ /* Simplify the code below by promoting OP_OUT to OP_INOUT in
predicated instructions, but only for register operands
that are already tracked, so that we can create a chain
when the first SET makes a register live. */
@@ -1595,8 +1594,6 @@ build_def_use (basic_block bb)
{
rtx op = recog_data.operand[i];
int matches = op_alt[i].matches;
- if (matches >= 0)
- op_alt[i].cl = op_alt[matches].cl;
if (matches >= 0 || op_alt[i].matched >= 0
|| (predicated && recog_data.operand_type[i] == OP_OUT))
{
@@ -1681,7 +1678,7 @@ build_def_use (basic_block bb)
rtx *loc = (i < n_ops
? recog_data.operand_loc[opn]
: recog_data.dup_loc[i - n_ops]);
- enum reg_class cl = op_alt[opn].cl;
+ enum reg_class cl = alternative_class (op_alt, opn);
enum op_type type = recog_data.operand_type[opn];
/* Don't scan match_operand here, since we've no reg class
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
2014-05-31 9:06 ` [PATCH 1/5] Flatten recog_op_alt and reorder entries Richard Sandiford
2014-05-31 9:09 ` [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints Richard Sandiford
@ 2014-05-31 9:15 ` Richard Sandiford
2014-06-03 21:58 ` Jeff Law
2014-05-31 9:17 ` [PATCH 4/5] Cache recog_op_alt by insn code: main patch Richard Sandiford
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:15 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
As described in the covering note, it seems better to put the onus of
checking the enabled attribute on the passes that are walking each
alternative, like LRA does for its internal subpasses. That leads
to a nicer interface in patch 4 and would make it easier to precompute
the information at build time. (The only thing preventing that now is
the subunion class.)
Thanks,
Richard
gcc/
* recog.c (preprocess_constraints): Don't skip disabled alternatives.
* ira-lives.c (check_and_make_def_conflict): Check for disabled
alternatives.
(make_early_clobber_and_input_conflicts): Likewise.
* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-05-31 08:57:57.642085058 +0100
+++ gcc/recog.c 2014-05-31 09:07:21.669714878 +0100
@@ -2352,12 +2352,6 @@ preprocess_constraints (void)
op_alt[i].matches = -1;
op_alt[i].matched = -1;
- if (!TEST_BIT (recog_data.enabled_alternatives, j))
- {
- p = skip_alternative (p);
- continue;
- }
-
if (*p == '\0' || *p == ',')
{
op_alt[i].anything_ok = 1;
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-05-31 08:54:12.238234755 +0100
+++ gcc/ira-lives.c 2014-05-31 09:07:21.670714886 +0100
@@ -641,8 +641,11 @@ check_and_make_def_conflict (int alt, in
/* If there's any alternative that allows USE to match DEF, do not
record a conflict. If that causes us to create an invalid
instruction due to the earlyclobber, reload must fix it up. */
+ alternative_mask enabled = recog_data.enabled_alternatives;
for (alt1 = 0; alt1 < recog_data.n_alternatives; alt1++)
{
+ if (!TEST_BIT (enabled, alt1))
+ continue;
operand_alternative *op_alt1 = &recog_op_alt[alt1 * n_operands];
if (op_alt1[use].matches == def
|| (use < n_operands - 1
@@ -688,30 +691,32 @@ make_early_clobber_and_input_conflicts (
int n_alternatives = recog_data.n_alternatives;
int n_operands = recog_data.n_operands;
+ alternative_mask enabled = recog_data.enabled_alternatives;
operand_alternative *op_alt = recog_op_alt;
for (alt = 0; alt < n_alternatives; alt++, op_alt += n_operands)
- for (def = 0; def < n_operands; def++)
- {
- def_cl = NO_REGS;
- if (op_alt[def].earlyclobber)
- {
- if (op_alt[def].anything_ok)
- def_cl = ALL_REGS;
- else
- def_cl = op_alt[def].cl;
- check_and_make_def_conflict (alt, def, def_cl);
- }
- if ((def_match = op_alt[def].matches) >= 0
- && (op_alt[def_match].earlyclobber
- || op_alt[def].earlyclobber))
- {
- if (op_alt[def_match].anything_ok)
- def_cl = ALL_REGS;
- else
- def_cl = op_alt[def_match].cl;
- check_and_make_def_conflict (alt, def, def_cl);
- }
- }
+ if (TEST_BIT (enabled, alt))
+ for (def = 0; def < n_operands; def++)
+ {
+ def_cl = NO_REGS;
+ if (op_alt[def].earlyclobber)
+ {
+ if (op_alt[def].anything_ok)
+ def_cl = ALL_REGS;
+ else
+ def_cl = op_alt[def].cl;
+ check_and_make_def_conflict (alt, def, def_cl);
+ }
+ if ((def_match = op_alt[def].matches) >= 0
+ && (op_alt[def_match].earlyclobber
+ || op_alt[def].earlyclobber))
+ {
+ if (op_alt[def_match].anything_ok)
+ def_cl = ALL_REGS;
+ else
+ def_cl = op_alt[def_match].cl;
+ check_and_make_def_conflict (alt, def, def_cl);
+ }
+ }
}
/* Mark early clobber hard registers of the current INSN as live (if
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c 2014-05-31 08:54:12.236234739 +0100
+++ gcc/config/i386/i386.c 2014-05-31 09:07:21.682714985 +0100
@@ -5874,8 +5874,11 @@ ix86_legitimate_combined_insn (rtx insn)
/* Operand has no constraints, anything is OK. */
win = !n_alternatives;
+ alternative_mask enabled = recog_data.enabled_alternatives;
for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
{
+ if (!TEST_BIT (enabled, j))
+ continue;
if (op_alt[i].anything_ok
|| (op_alt[i].matches != -1
&& operands_match_p
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/5] Cache recog_op_alt by insn code: main patch
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
` (2 preceding siblings ...)
2014-05-31 9:15 ` [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute Richard Sandiford
@ 2014-05-31 9:17 ` Richard Sandiford
2014-06-03 22:01 ` Jeff Law
2014-05-31 9:23 ` [PATCH 5/5] Reuse recog_op_alt cache in LRA Richard Sandiford
2014-06-03 21:38 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Jeff Law
5 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:17 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
This is the refreshed main patch. I've rearranged the preprocess_constraints
code into three functions so that LRA can use it in patch 5.
Thanks,
Richard
gcc/
* recog.h (operand_alternative): Convert reg_class, reject,
matched and matches into bitfields.
(preprocess_constraints): New overload.
(preprocess_insn_constraints): New function.
(preprocess_constraints): Take the insn as parameter.
(recog_op_alt): Change into a pointer.
(target_recog): Add x_op_alt.
* recog.c (asm_op_alt): New variable.
(recog_op_alt): Change into a pointer.
(preprocess_constraints): New overload, replacing the old function
definition with one that doesn't use global state.
(preprocess_insn_constraints): New function.
(preprocess_constraints): Use them. Take the insn as parameter.
Use asm_op_alt for asms.
(recog_init): Free existing x_op_alt entries.
* ira-lives.c (check_and_make_def_conflict): Make operand_alternative
pointer const.
(make_early_clobber_and_input_conflicts): Likewise.
(process_bb_node_lives): Pass the insn to process_constraints.
* reg-stack.c (check_asm_stack_operands): Likewise.
(subst_asm_stack_regs): Likewise.
* regcprop.c (copyprop_hardreg_forward_1): Likewise.
* regrename.c (build_def_use): Likewise.
* sched-deps.c (sched_analyze_insn): Likewise.
* sel-sched.c (get_reg_class, implicit_clobber_conflict_p): Likewise.
* config/arm/arm.c (xscale_sched_adjust_cost): Likewise.
(note_invalid_constants): Likewise.
* config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
(ix86_legitimate_combined_insn): Make operand_alternative pointer
const.
Index: gcc/recog.h
===================================================================
--- gcc/recog.h 2014-05-31 09:02:35.956369716 +0100
+++ gcc/recog.h 2014-05-31 09:10:16.596150618 +0100
@@ -46,18 +46,18 @@ struct operand_alternative
const char *constraint;
/* The register class valid for this alternative (possibly NO_REGS). */
- enum reg_class cl;
+ ENUM_BITFIELD (reg_class) cl : 16;
/* "Badness" of this alternative, computed from number of '?' and '!'
characters in the constraint string. */
- unsigned int reject;
+ unsigned int reject : 16;
/* -1 if no matching constraint was found, or an operand number. */
- int matches;
+ int matches : 8;
/* The same information, but reversed: -1 if this operand is not
matched by any other, or the operand number of the operand that
matches this one. */
- int matched;
+ int matched : 8;
/* Nonzero if '&' was found in the constraint string. */
unsigned int earlyclobber:1;
@@ -77,6 +77,8 @@ struct operand_alternative
/* Nonzero if 'X' was found in the constraint string, or if the constraint
string for this alternative was empty. */
unsigned int anything_ok:1;
+
+ unsigned int unused : 8;
};
/* Return the class for operand I of alternative ALT, taking matching
@@ -142,7 +144,10 @@ extern void insn_extract (rtx);
extern void extract_insn (rtx);
extern void extract_constrain_insn_cached (rtx);
extern void extract_insn_cached (rtx);
-extern void preprocess_constraints (void);
+extern void preprocess_constraints (int, int, const char **,
+ operand_alternative *);
+extern const operand_alternative *preprocess_insn_constraints (int);
+extern void preprocess_constraints (rtx);
extern rtx peep2_next_insn (int);
extern int peep2_regno_dead_p (int, int);
extern int peep2_reg_dead_p (int, rtx);
@@ -264,8 +269,7 @@ struct recog_data_d
extern struct recog_data_d recog_data;
-extern struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS
- * MAX_RECOG_ALTERNATIVES];
+extern const operand_alternative *recog_op_alt;
/* Return a pointer to an array in which index OP describes the constraints
on operand OP of the current instruction alternative (which_alternative).
@@ -396,6 +400,7 @@ struct insn_data_d
struct target_recog {
bool x_initialized;
alternative_mask x_enabled_alternatives[LAST_INSN_CODE];
+ operand_alternative *x_op_alt[LAST_INSN_CODE];
};
extern struct target_recog default_target_recog;
Index: gcc/recog.c
===================================================================
--- gcc/recog.c 2014-05-31 09:07:21.669714878 +0100
+++ gcc/recog.c 2014-05-31 09:15:38.746800685 +0100
@@ -81,8 +81,11 @@ struct recog_data_d recog_data;
/* Contains a vector of operand_alternative structures, such that
operand OP of alternative A is at index A * n_operands + OP.
Set up by preprocess_constraints. */
-struct operand_alternative recog_op_alt[MAX_RECOG_OPERANDS
- * MAX_RECOG_ALTERNATIVES];
+const operand_alternative *recog_op_alt;
+
+/* Used to provide recog_op_alt for asms. */
+static operand_alternative asm_op_alt[MAX_RECOG_OPERANDS
+ * MAX_RECOG_ALTERNATIVES];
/* On return from `constrain_operands', indicate which alternative
was satisfied. */
@@ -2324,26 +2327,23 @@ extract_insn (rtx insn)
which_alternative = -1;
}
-/* After calling extract_insn, you can use this function to extract some
- information from the constraint strings into a more usable form.
- The collected data is stored in recog_op_alt. */
+/* Fill in OP_ALT_BASE for an instruction that has N_OPERANDS operands,
+ N_ALTERNATIVES alternatives and constraint strings CONSTRAINTS.
+ OP_ALT_BASE has N_ALTERNATIVES * N_OPERANDS entries and CONSTRAINTS
+ has N_OPERANDS entries. */
+
void
-preprocess_constraints (void)
+preprocess_constraints (int n_operands, int n_alternatives,
+ const char **constraints,
+ operand_alternative *op_alt_base)
{
- int i;
-
- int n_operands = recog_data.n_operands;
- int n_alternatives = recog_data.n_alternatives;
- int n_entries = n_operands * n_alternatives;
- memset (recog_op_alt, 0, n_entries * sizeof (struct operand_alternative));
-
- for (i = 0; i < n_operands; i++)
+ for (int i = 0; i < n_operands; i++)
{
int j;
struct operand_alternative *op_alt;
- const char *p = recog_data.constraints[i];
+ const char *p = constraints[i];
- op_alt = recog_op_alt;
+ op_alt = op_alt_base;
for (j = 0; j < n_alternatives; j++, op_alt += n_operands)
{
@@ -2462,6 +2462,59 @@ preprocess_constraints (void)
}
}
+/* Return an array of operand_alternative instructions for
+ instruction ICODE. */
+
+const operand_alternative *
+preprocess_insn_constraints (int icode)
+{
+ gcc_checking_assert (IN_RANGE (icode, 0, LAST_INSN_CODE));
+ if (this_target_recog->x_op_alt[icode])
+ return this_target_recog->x_op_alt[icode];
+
+ int n_operands = insn_data[icode].n_operands;
+ if (n_operands == 0)
+ return 0;
+ /* Always provide at least one alternative so that which_op_alt ()
+ works correctly. If the instruction has 0 alternatives (i.e. all
+ constraint strings are empty) then each operand in this alternative
+ will have anything_ok set. */
+ int n_alternatives = MAX (insn_data[icode].n_alternatives, 1);
+ int n_entries = n_operands * n_alternatives;
+
+ operand_alternative *op_alt = XCNEWVEC (operand_alternative, n_entries);
+ const char **constraints = XALLOCAVEC (const char *, n_operands);
+
+ for (int i = 0; i < n_operands; ++i)
+ constraints[i] = insn_data[icode].operand[i].constraint;
+ preprocess_constraints (n_operands, n_alternatives, constraints, op_alt);
+
+ this_target_recog->x_op_alt[icode] = op_alt;
+ return op_alt;
+}
+
+/* After calling extract_insn, you can use this function to extract some
+ information from the constraint strings into a more usable form.
+ The collected data is stored in recog_op_alt. */
+
+void
+preprocess_constraints (rtx insn)
+{
+ int icode = INSN_CODE (insn);
+ if (icode >= 0)
+ recog_op_alt = preprocess_insn_constraints (icode);
+ else
+ {
+ int n_operands = recog_data.n_operands;
+ int n_alternatives = recog_data.n_alternatives;
+ int n_entries = n_operands * n_alternatives;
+ memset (asm_op_alt, 0, n_entries * sizeof (operand_alternative));
+ preprocess_constraints (n_operands, n_alternatives,
+ recog_data.constraints, asm_op_alt);
+ recog_op_alt = asm_op_alt;
+ }
+}
+
/* Check the operands of an insn against the insn's operand constraints
and return 1 if they are valid.
The information about the insn's operands, constraints, operand modes
@@ -4211,4 +4264,10 @@ recog_init ()
}
memset (this_target_recog->x_enabled_alternatives, 0,
sizeof (this_target_recog->x_enabled_alternatives));
+ for (int i = 0; i < LAST_INSN_CODE; ++i)
+ if (this_target_recog->x_op_alt[i])
+ {
+ free (this_target_recog->x_op_alt[i]);
+ this_target_recog->x_op_alt[i] = 0;
+ }
}
Index: gcc/ira-lives.c
===================================================================
--- gcc/ira-lives.c 2014-05-31 09:07:21.670714886 +0100
+++ gcc/ira-lives.c 2014-05-31 09:10:16.598150634 +0100
@@ -625,7 +625,7 @@ check_and_make_def_conflict (int alt, in
advance_p = true;
int n_operands = recog_data.n_operands;
- operand_alternative *op_alt = &recog_op_alt[alt * n_operands];
+ const operand_alternative *op_alt = &recog_op_alt[alt * n_operands];
for (use = 0; use < n_operands; use++)
{
int alt1;
@@ -646,7 +646,8 @@ check_and_make_def_conflict (int alt, in
{
if (!TEST_BIT (enabled, alt1))
continue;
- operand_alternative *op_alt1 = &recog_op_alt[alt1 * n_operands];
+ const operand_alternative *op_alt1
+ = &recog_op_alt[alt1 * n_operands];
if (op_alt1[use].matches == def
|| (use < n_operands - 1
&& recog_data.constraints[use][0] == '%'
@@ -692,7 +693,7 @@ make_early_clobber_and_input_conflicts (
int n_alternatives = recog_data.n_alternatives;
int n_operands = recog_data.n_operands;
alternative_mask enabled = recog_data.enabled_alternatives;
- operand_alternative *op_alt = recog_op_alt;
+ const operand_alternative *op_alt = recog_op_alt;
for (alt = 0; alt < n_alternatives; alt++, op_alt += n_operands)
if (TEST_BIT (enabled, alt))
for (def = 0; def < n_operands; def++)
@@ -1251,7 +1252,7 @@ process_bb_node_lives (ira_loop_tree_nod
}
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
process_single_reg_class_operands (false, freq);
/* See which defined values die here. */
Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c 2014-05-31 09:02:35.968369814 +0100
+++ gcc/reg-stack.c 2014-05-31 09:10:16.601150659 +0100
@@ -471,7 +471,7 @@ check_asm_stack_operands (rtx insn)
extract_insn (insn);
constrain_operands (1);
- preprocess_constraints ();
+ preprocess_constraints (insn);
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
@@ -2029,7 +2029,7 @@ subst_asm_stack_regs (rtx insn, stack_pt
extract_insn (insn);
constrain_operands (1);
- preprocess_constraints ();
+ preprocess_constraints (insn);
const operand_alternative *op_alt = which_op_alt ();
get_asm_operands_in_out (body, &n_outputs, &n_inputs);
Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c 2014-05-31 09:02:35.957369724 +0100
+++ gcc/regcprop.c 2014-05-31 09:10:16.601150659 +0100
@@ -774,7 +774,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
const operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
is_asm = asm_noperands (PATTERN (insn)) >= 0;
@@ -877,7 +877,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
}
/* Otherwise, try all valid registers and see if its valid. */
@@ -905,7 +905,7 @@ copyprop_hardreg_forward_1 (basic_block
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
}
}
}
Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c 2014-05-31 09:02:35.958369732 +0100
+++ gcc/regrename.c 2014-05-31 09:10:16.602150667 +0100
@@ -1570,7 +1570,7 @@ build_def_use (basic_block bb)
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
const operand_alternative *op_alt = which_op_alt ();
n_ops = recog_data.n_operands;
untracked_operands = 0;
Index: gcc/sched-deps.c
===================================================================
--- gcc/sched-deps.c 2014-05-31 08:54:09.086208879 +0100
+++ gcc/sched-deps.c 2014-05-31 09:10:16.615150774 +0100
@@ -2865,7 +2865,7 @@ sched_analyze_insn (struct deps_desc *de
HARD_REG_SET temp;
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
ira_implicitly_set_insn_hard_regs (&temp);
AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
IOR_HARD_REG_SET (implicit_reg_pending_clobbers, temp);
Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c 2014-05-31 09:02:35.970369830 +0100
+++ gcc/sel-sched.c 2014-05-31 09:10:16.617150790 +0100
@@ -1019,7 +1019,7 @@ get_reg_class (rtx insn)
extract_insn (insn);
if (! constrain_operands (1))
fatal_insn_not_found (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
n_ops = recog_data.n_operands;
const operand_alternative *op_alt = which_op_alt ();
@@ -2134,7 +2134,7 @@ implicit_clobber_conflict_p (insn_t thro
/* Calculate implicit clobbers. */
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
ira_implicitly_set_insn_hard_regs (&temp);
AND_COMPL_HARD_REG_SET (temp, ira_no_alloc_regs);
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c 2014-05-31 09:02:35.966369798 +0100
+++ gcc/config/arm/arm.c 2014-05-31 09:10:16.626150864 +0100
@@ -11335,7 +11335,7 @@ xscale_sched_adjust_cost (rtx insn, rtx
that overlaps with SHIFTED_OPERAND, then we have increase the
cost of this dependency. */
extract_insn (dep);
- preprocess_constraints ();
+ preprocess_constraints (dep);
for (opno = 0; opno < recog_data.n_operands; opno++)
{
/* We can ignore strict inputs. */
@@ -16870,7 +16870,7 @@ note_invalid_constants (rtx insn, HOST_W
/* Fill in recog_op_alt with information about the constraints of
this insn. */
- preprocess_constraints ();
+ preprocess_constraints (insn);
const operand_alternative *op_alt = which_op_alt ();
for (opno = 0; opno < recog_data.n_operands; opno++)
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c 2014-05-31 09:07:21.682714985 +0100
+++ gcc/config/i386/i386.c 2014-05-31 09:10:16.638150962 +0100
@@ -5826,7 +5826,7 @@ ix86_legitimate_combined_insn (rtx insn)
int i;
extract_insn (insn);
- preprocess_constraints ();
+ preprocess_constraints (insn);
int n_operands = recog_data.n_operands;
int n_alternatives = recog_data.n_alternatives;
@@ -5834,7 +5834,7 @@ ix86_legitimate_combined_insn (rtx insn)
{
rtx op = recog_data.operand[i];
enum machine_mode mode = GET_MODE (op);
- operand_alternative *op_alt;
+ const operand_alternative *op_alt;
int offset = 0;
bool win;
int j;
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/5] Reuse recog_op_alt cache in LRA
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
` (3 preceding siblings ...)
2014-05-31 9:17 ` [PATCH 4/5] Cache recog_op_alt by insn code: main patch Richard Sandiford
@ 2014-05-31 9:23 ` Richard Sandiford
2014-06-03 22:02 ` Jeff Law
2014-06-03 21:38 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Jeff Law
5 siblings, 1 reply; 20+ messages in thread
From: Richard Sandiford @ 2014-05-31 9:23 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
The operand_alternative cache was LRA's only per-subtarget state
so a lot of this patch is removing the associated initialisation
and target_globals stuff.
I called the preprocess_constraints functions in lra_set_insn_recog_data
rather than setup_operand_alternative because lra_set_insn_recog_data
already has a local constraints array (needed for asms).
setup_operand_alternative is now only called in the uncached case.
Thanks,
Richard
gcc/
* lra-int.h (lra_static_insn_data): Make operand_alternative a
const pointer.
(target_lra_int, default_target_lra_int, this_target_lra_int)
(op_alt_data): Delete.
* lra.h (lra_init): Delete.
* lra.c (default_target_lra_int, this_target_lra_int): Delete.
(init_insn_code_data_once): Remove op_alt_data handling.
(finish_insn_code_data_once): Likewise.
(init_op_alt_data): Delete.
(get_static_insn_data): Initialize operand_alternative to null.
(free_insn_recog_data): Cast operand_alternative before freeing it.
(setup_operand_alternative): Take the operand_alternative as
parameter and assume it isn't already cached in the static
insn data.
(lra_set_insn_recog_data): Update accordingly.
(lra_init): Delete.
* ira.c (ira_init): Don't call lra_init.
* target-globals.h (this_target_lra_int): Declare.
(target_globals): Remove lra_int.
(restore_target_globals): Update accordingly.
* target-globals.c: Don't include lra-int.h.
(default_target_globals, save_target_globals): Remove lra_int.
Index: gcc/lra-int.h
===================================================================
--- gcc/lra-int.h 2014-05-31 08:54:08.894207303 +0100
+++ gcc/lra-int.h 2014-05-31 09:25:56.876885502 +0100
@@ -198,7 +198,7 @@ struct lra_static_insn_data
/* Array [n_alternatives][n_operand] of static constraint info for
given operand in given alternative. This info can be changed if
the target reg info is changed. */
- struct operand_alternative *operand_alternative;
+ const struct operand_alternative *operand_alternative;
};
/* LRA internal info about an insn (LRA internal insn
@@ -495,21 +495,3 @@ lra_assign_reg_val (int from, int to)
lra_reg_info[to].val = lra_reg_info[from].val;
lra_reg_info[to].offset = lra_reg_info[from].offset;
}
-\f
-
-struct target_lra_int
-{
- /* Map INSN_UID -> the operand alternative data (NULL if unknown).
- We assume that this data is valid until register info is changed
- because classes in the data can be changed. */
- struct operand_alternative *x_op_alt_data[LAST_INSN_CODE];
-};
-
-extern struct target_lra_int default_target_lra_int;
-#if SWITCHABLE_TARGET
-extern struct target_lra_int *this_target_lra_int;
-#else
-#define this_target_lra_int (&default_target_lra_int)
-#endif
-
-#define op_alt_data (this_target_lra_int->x_op_alt_data)
Index: gcc/lra.h
===================================================================
--- gcc/lra.h 2014-05-31 08:54:08.894207303 +0100
+++ gcc/lra.h 2014-05-31 09:25:56.951886120 +0100
@@ -36,5 +36,4 @@ extern rtx lra_create_new_reg (enum mach
extern rtx lra_eliminate_regs (rtx, enum machine_mode, rtx);
extern void lra (FILE *);
extern void lra_init_once (void);
-extern void lra_init (void);
extern void lra_finish_once (void);
Index: gcc/lra.c
===================================================================
--- gcc/lra.c 2014-05-31 08:54:08.894207303 +0100
+++ gcc/lra.c 2014-05-31 09:25:56.891885625 +0100
@@ -553,11 +553,6 @@ finish_insn_regs (void)
/* This page contains code dealing LRA insn info (or in other words
LRA internal insn representation). */
-struct target_lra_int default_target_lra_int;
-#if SWITCHABLE_TARGET
-struct target_lra_int *this_target_lra_int = &default_target_lra_int;
-#endif
-
/* Map INSN_CODE -> the static insn data. This info is valid during
all translation unit. */
struct lra_static_insn_data *insn_code_data[LAST_INSN_CODE];
@@ -599,7 +594,6 @@ static struct lra_static_insn_data debug
init_insn_code_data_once (void)
{
memset (insn_code_data, 0, sizeof (insn_code_data));
- memset (op_alt_data, 0, sizeof (op_alt_data));
}
/* Called once per compiler work to finalize some LRA data related to
@@ -613,25 +607,9 @@ finish_insn_code_data_once (void)
{
if (insn_code_data[i] != NULL)
free (insn_code_data[i]);
- if (op_alt_data[i] != NULL)
- free (op_alt_data[i]);
}
}
-/* Initialize LRA info about operands in insn alternatives. */
-static void
-init_op_alt_data (void)
-{
- int i;
-
- for (i = 0; i < LAST_INSN_CODE; i++)
- if (op_alt_data[i] != NULL)
- {
- free (op_alt_data[i]);
- op_alt_data[i] = NULL;
- }
-}
-
/* Return static insn data, allocate and setup if necessary. Although
dup_num is static data (it depends only on icode), to set it up we
need to extract insn first. So recog_data should be valid for
@@ -650,6 +628,7 @@ get_static_insn_data (int icode, int nop
+ sizeof (struct lra_operand_data) * nop
+ sizeof (int) * ndup;
data = XNEWVAR (struct lra_static_insn_data, n_bytes);
+ data->operand_alternative = NULL;
data->n_operands = nop;
data->n_dups = ndup;
data->n_alternatives = nalt;
@@ -727,7 +706,8 @@ free_insn_recog_data (lra_insn_recog_dat
if (data->icode < 0 && NONDEBUG_INSN_P (data->insn))
{
if (data->insn_static_data->operand_alternative != NULL)
- free (data->insn_static_data->operand_alternative);
+ free (const_cast <operand_alternative *>
+ (data->insn_static_data->operand_alternative));
free_insn_regs (data->insn_static_data->hard_regs);
free (data->insn_static_data);
}
@@ -752,173 +732,38 @@ finish_insn_recog_data (void)
/* Setup info about operands in alternatives of LRA DATA of insn. */
static void
-setup_operand_alternative (lra_insn_recog_data_t data)
+setup_operand_alternative (lra_insn_recog_data_t data,
+ const operand_alternative *op_alt)
{
- int i, nop, nalt;
+ int i, j, nop, nalt;
int icode = data->icode;
struct lra_static_insn_data *static_data = data->insn_static_data;
- if (icode >= 0
- && (static_data->operand_alternative = op_alt_data[icode]) != NULL)
- return;
static_data->commutative = -1;
nop = static_data->n_operands;
- if (nop == 0)
- {
- static_data->operand_alternative = NULL;
- return;
- }
nalt = static_data->n_alternatives;
- static_data->operand_alternative = XNEWVEC (struct operand_alternative,
- nalt * nop);
- memset (static_data->operand_alternative, 0,
- nalt * nop * sizeof (struct operand_alternative));
- if (icode >= 0)
- op_alt_data[icode] = static_data->operand_alternative;
+ static_data->operand_alternative = op_alt;
for (i = 0; i < nop; i++)
{
- int j;
- struct operand_alternative *op_alt_start, *op_alt;
- const char *p = static_data->operand[i].constraint;
-
- static_data->operand[i].early_clobber = 0;
- op_alt_start = &static_data->operand_alternative[i];
-
- for (j = 0; j < nalt; j++)
- {
- op_alt = op_alt_start + j * nop;
- op_alt->cl = NO_REGS;
- op_alt->constraint = p;
- op_alt->matches = -1;
- op_alt->matched = -1;
-
- if (*p == '\0' || *p == ',')
- {
- op_alt->anything_ok = 1;
- continue;
- }
-
- for (;;)
- {
- char c = *p;
- if (c == '#')
- do
- c = *++p;
- while (c != ',' && c != '\0');
- if (c == ',' || c == '\0')
- {
- p++;
- break;
- }
-
- switch (c)
- {
- case '=': case '+': case '*':
- case 'E': case 'F': case 'G': case 'H':
- case 's': case 'i': case 'n':
- case 'I': case 'J': case 'K': case 'L':
- case 'M': case 'N': case 'O': case 'P':
- /* These don't say anything we care about. */
- break;
-
- case '%':
- /* We currently only support one commutative pair of
- operands. */
- if (static_data->commutative < 0)
- static_data->commutative = i;
- else
- lra_assert (data->icode < 0); /* Asm */
-
- /* The last operand should not be marked
- commutative. */
- lra_assert (i != nop - 1);
- break;
-
- case '?':
- op_alt->reject += LRA_LOSER_COST_FACTOR;
- break;
- case '!':
- op_alt->reject += LRA_MAX_REJECT;
- break;
- case '&':
- op_alt->earlyclobber = 1;
- static_data->operand[i].early_clobber = 1;
- break;
-
- case '0': case '1': case '2': case '3': case '4':
- case '5': case '6': case '7': case '8': case '9':
- {
- char *end;
- op_alt->matches = strtoul (p, &end, 10);
- static_data->operand_alternative
- [j * nop + op_alt->matches].matched = i;
- p = end;
- }
- continue;
-
- case TARGET_MEM_CONSTRAINT:
- op_alt->memory_ok = 1;
- break;
- case '<':
- op_alt->decmem_ok = 1;
- break;
- case '>':
- op_alt->incmem_ok = 1;
- break;
- case 'V':
- op_alt->nonoffmem_ok = 1;
- break;
- case 'o':
- op_alt->offmem_ok = 1;
- break;
- case 'X':
- op_alt->anything_ok = 1;
- break;
-
- case 'p':
- static_data->operand[i].is_address = true;
- op_alt->is_address = 1;
- op_alt->cl = (reg_class_subunion[(int) op_alt->cl]
- [(int) base_reg_class (VOIDmode,
- ADDR_SPACE_GENERIC,
- ADDRESS, SCRATCH)]);
- break;
-
- case 'g':
- case 'r':
- op_alt->cl =
- reg_class_subunion[(int) op_alt->cl][(int) GENERAL_REGS];
- break;
-
- default:
- if (EXTRA_MEMORY_CONSTRAINT (c, p))
- {
- op_alt->memory_ok = 1;
- break;
- }
- if (EXTRA_ADDRESS_CONSTRAINT (c, p))
- {
- static_data->operand[i].is_address = true;
- op_alt->is_address = 1;
- op_alt->cl
- = (reg_class_subunion
- [(int) op_alt->cl]
- [(int) base_reg_class (VOIDmode, ADDR_SPACE_GENERIC,
- ADDRESS, SCRATCH)]);
- break;
- }
-
- op_alt->cl
- = (reg_class_subunion
- [(int) op_alt->cl]
- [(int)
- REG_CLASS_FROM_CONSTRAINT ((unsigned char) c, p)]);
- break;
- }
- p += CONSTRAINT_LEN (c, p);
- }
+ static_data->operand[i].early_clobber = false;
+ static_data->operand[i].is_address = false;
+ if (static_data->operand[i].constraint[0] == '%')
+ {
+ /* We currently only support one commutative pair of operands. */
+ if (static_data->commutative < 0)
+ static_data->commutative = i;
+ else
+ lra_assert (icode < 0); /* Asm */
+ /* The last operand should not be marked commutative. */
+ lra_assert (i != nop - 1);
}
}
+ for (j = 0; j < nalt; j++)
+ for (i = 0; i < nop; i++, op_alt++)
+ {
+ static_data->operand[i].early_clobber |= op_alt->earlyclobber;
+ static_data->operand[i].is_address |= op_alt->is_address;
+ }
}
/* Recursively process X and collect info about registers, which are
@@ -1077,12 +922,13 @@ lra_set_insn_recog_data (rtx insn)
}
if (icode < 0)
{
- int nop;
+ int nop, nalt;
enum machine_mode operand_mode[MAX_RECOG_OPERANDS];
const char *constraints[MAX_RECOG_OPERANDS];
nop = asm_noperands (PATTERN (insn));
data->operand_loc = data->dup_loc = NULL;
+ nalt = 1;
if (nop < 0)
{
/* Its is a special insn like USE or CLOBBER. We should
@@ -1092,7 +938,7 @@ lra_set_insn_recog_data (rtx insn)
|| GET_CODE (PATTERN (insn)) == CLOBBER
|| GET_CODE (PATTERN (insn)) == ASM_INPUT);
data->insn_static_data = insn_static_data
- = get_static_insn_data (-1, 0, 0, 1);
+ = get_static_insn_data (-1, 0, 0, nalt);
}
else
{
@@ -1106,16 +952,15 @@ lra_set_insn_recog_data (rtx insn)
decode_asm_operands (PATTERN (insn), NULL,
data->operand_loc,
constraints, operand_mode, NULL);
- n = 1;
if (nop > 0)
{
const char *p = recog_data.constraints[0];
for (p = constraints[0]; *p; p++)
- n += *p == ',';
+ nalt += *p == ',';
}
data->insn_static_data = insn_static_data
- = get_static_insn_data (-1, nop, 0, n);
+ = get_static_insn_data (-1, nop, 0, nalt);
for (i = 0; i < nop; i++)
{
insn_static_data->operand[i].mode = operand_mode[i];
@@ -1131,6 +976,13 @@ lra_set_insn_recog_data (rtx insn)
: insn_static_data->operand[i].constraint[0] == '+' ? OP_INOUT
: OP_IN);
data->enabled_alternatives = ALL_ALTERNATIVES;
+ if (nop > 0)
+ {
+ operand_alternative *op_alt = XCNEWVEC (operand_alternative,
+ nalt * nop);
+ preprocess_constraints (nop, nalt, constraints, op_alt);
+ setup_operand_alternative (data, op_alt);
+ }
}
else
{
@@ -1158,6 +1010,11 @@ lra_set_insn_recog_data (rtx insn)
}
data->dup_loc = locs;
data->enabled_alternatives = get_enabled_alternatives (insn);
+ const operand_alternative *op_alt = preprocess_insn_constraints (icode);
+ if (!insn_static_data->operand_alternative)
+ setup_operand_alternative (data, op_alt);
+ else if (op_alt != insn_static_data->operand_alternative)
+ insn_static_data->operand_alternative = op_alt;
}
if (GET_CODE (PATTERN (insn)) == CLOBBER || GET_CODE (PATTERN (insn)) == USE)
insn_static_data->hard_regs = NULL;
@@ -1165,7 +1022,6 @@ lra_set_insn_recog_data (rtx insn)
insn_static_data->hard_regs
= collect_non_operand_hard_regs (&PATTERN (insn), data,
NULL, OP_IN, false);
- setup_operand_alternative (data);
data->arg_hard_regs = NULL;
if (CALL_P (insn))
{
@@ -2451,13 +2307,6 @@ lra_init_once (void)
init_insn_code_data_once ();
}
-/* Initialize LRA whenever register-related information is changed. */
-void
-lra_init (void)
-{
- init_op_alt_data ();
-}
-
/* Called once per compiler to finish LRA data which are initialize
once. */
void
Index: gcc/ira.c
===================================================================
--- gcc/ira.c 2014-05-31 08:54:08.894207303 +0100
+++ gcc/ira.c 2014-05-31 09:25:56.934885982 +0100
@@ -1715,7 +1715,6 @@ ira_init (void)
clarify_prohibited_class_mode_regs ();
setup_hard_regno_aclass ();
ira_init_costs ();
- lra_init ();
}
/* Function called once at the end of compiler work. */
Index: gcc/target-globals.h
===================================================================
--- gcc/target-globals.h 2014-05-31 08:54:08.894207303 +0100
+++ gcc/target-globals.h 2014-05-31 09:25:56.914885815 +0100
@@ -33,7 +33,6 @@ #define TARGET_GLOBALS_H 1
extern struct target_cfgloop *this_target_cfgloop;
extern struct target_ira *this_target_ira;
extern struct target_ira_int *this_target_ira_int;
-extern struct target_lra_int *this_target_lra_int;
extern struct target_builtins *this_target_builtins;
extern struct target_gcse *this_target_gcse;
extern struct target_bb_reorder *this_target_bb_reorder;
@@ -53,7 +52,6 @@ struct GTY(()) target_globals {
struct target_cfgloop *GTY((skip)) cfgloop;
void *GTY((atomic)) ira;
void *GTY((atomic)) ira_int;
- void *GTY((atomic)) lra_int;
struct target_builtins *GTY((skip)) builtins;
struct target_gcse *GTY((skip)) gcse;
struct target_bb_reorder *GTY((skip)) bb_reorder;
@@ -81,7 +79,6 @@ restore_target_globals (struct target_gl
this_target_cfgloop = g->cfgloop;
this_target_ira = (struct target_ira *) g->ira;
this_target_ira_int = (struct target_ira_int *) g->ira_int;
- this_target_lra_int = (struct target_lra_int *) g->lra_int;
this_target_builtins = g->builtins;
this_target_gcse = g->gcse;
this_target_bb_reorder = g->bb_reorder;
Index: gcc/target-globals.c
===================================================================
--- gcc/target-globals.c 2014-05-31 08:54:08.894207303 +0100
+++ gcc/target-globals.c 2014-05-31 09:25:56.909885775 +0100
@@ -38,7 +38,6 @@ Software Foundation; either version 3, o
#include "libfuncs.h"
#include "cfgloop.h"
#include "ira-int.h"
-#include "lra-int.h"
#include "builtins.h"
#include "gcse.h"
#include "bb-reorder.h"
@@ -59,7 +58,6 @@ struct target_globals default_target_glo
&default_target_cfgloop,
&default_target_ira,
&default_target_ira_int,
- &default_target_lra_int,
&default_target_builtins,
&default_target_gcse,
&default_target_bb_reorder,
@@ -96,7 +94,6 @@ save_target_globals (void)
g->cfgloop = &p->cfgloop;
g->ira = ggc_internal_cleared_alloc (sizeof (struct target_ira));
g->ira_int = ggc_internal_cleared_alloc (sizeof (struct target_ira_int));
- g->lra_int = ggc_internal_cleared_alloc (sizeof (struct target_lra_int));
g->builtins = &p->builtins;
g->gcse = &p->gcse;
g->bb_reorder = &p->bb_reorder;
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Cache recog_op_alt by insn code, take 2
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
` (4 preceding siblings ...)
2014-05-31 9:23 ` [PATCH 5/5] Reuse recog_op_alt cache in LRA Richard Sandiford
@ 2014-06-03 21:38 ` Jeff Law
2014-06-04 17:39 ` Richard Sandiford
5 siblings, 1 reply; 20+ messages in thread
From: Jeff Law @ 2014-06-03 21:38 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:02, Richard Sandiford wrote:
> Sorry Jeff, while working on the follow-on LRA patch I came across
> a couple of problems, so I need another round on this.
It happens, particularly for conceptual changes like this. I don't
consider that a problem at all -- we're already in agreement on the
overall direction this work is going, the rest is the implementation
details. With your long history with GCC, you get a lot of trust and
leeway.
> First of all, I mentioned doing a follow-on patch to make LRA and
> recog use the same cache for operand_alternatives. I hadn't realised
> until I wrote that patch that there's a significant difference between
> the LRA and recog_op_alt versions of the information: recog_op_alt
> groups alternatives together by operand whereas LRA groups operands
> together by alternative. So in the cached flat array, recog_op_alt
> uses [op * n_alternatives + alt] whereas LRA uses [alt * n_operands + nop].
> The two could only share a cache if they used the same order.
The kind of thing that becomes obvious once you try to make it work :-)
>
> I think the LRA ordering makes more sense. Quite a few places are
> only interested in alternative which_alternative, so grouping operands
> together by alternative gives a natural subarray. And there are places
> in IRA (which uses recog_op_alt rather than the LRA information) where
> the "for each alternative" loop is the outer one.
Yea, I would think that generally we're going to be interested in major
indexing by which_alternative rather than the operands.
>
> A second difference was that preprocess_constraints skips disabled
> alternatives while LRA's setup_operand_alternative doesn't; LRA just
> checks for disabled alternatives when walking the array instead.
> That should make no difference in practice, but since the LRA approach
> would also make it easier to precompute the array at build time,
> I thought it would be a better way to go. It also gives a cleaner
> interface: we can have a preprocess_insn_constraints function that
> takes a (define_insn-based) insn and returns the operand_alternative
> information without any global state.
Sounds good. Do you think prebuilding those arrays once at build time
is likely to have a measurable impact? I realize you probably haven't
done measurements, just looking for your gut instinct here.
>
> Also, it turns out that sel-sched.c, regcprop.c and regrename.c
> modify the recog_op_alt structure as a convenient way of handling
> matching constraints. That obviously isn't a good idea if we want
> other passes to reuse the data later. I've fixed that and made the
> types "const" so that new instances shouldn't creep in.
Ick. But glad to see the introduction of const to prevent this from
happening again.
>
> Also, I hadn't realised that a define_insn with no constraints
> at all has 0 alternatives rather than 1. Some passes nevertheless
> unconditionally access the recog_op_alt information for alternative
> which_alternative.
>
> I could have fixed that by making n_alternatives always be >= 1
> in the static insn table. Several places do have fast paths for
> n_alternatives == 0 though, so I thought it would be better to
> handle the 0 alternative case in preprocess_constraints as well.
> All it needs is a MIN (..., 1).
Funny thing is I suspect the n_alternatives == 0 case is rare in
practice though.
>>> - "matched" and "matches" are operand numbers and so fit in 8 bits
>> OK. This could also be smaller, don't we have an upper limit of 32 (or
>> 30?) on the number of operands appearing in an insn. It'd be a way to
>> get a few more bits if we need them someday.
>
> Yeah, we could probably squeeze everything into a single int if we needed
> to and if we were prepared to have non-byte-sized integer fields.
> Going from 2 ints to 1 int wouldn't help LP64 hosts as things stand
> though, since we have a pointer at the beginning of the struct.
Right and non-LP64 hosts are becoming more and more rare. And the
masking necessary may eat up any benefits we'd get, hard to know either way.
>
> Boostrapped & regression-tested on x86_64-linux-gnu. Also tested by
> building gcc.dg, g++.dg and gcc.c-torture for one target per config/
> directory and making sure that there were no asm differences.
Hoping to slog through them this afternoon. Time is a bit tight though.
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/5] Flatten recog_op_alt and reorder entries
2014-05-31 9:06 ` [PATCH 1/5] Flatten recog_op_alt and reorder entries Richard Sandiford
@ 2014-06-03 21:50 ` Jeff Law
0 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2014-06-03 21:50 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:06, Richard Sandiford wrote:
> As described in the covering note, this patch converts recog_op_alt
> into a flat array and orders the entries in the same way as the
> LRA cache. Several places just want the information for alternative
> which_alternative, so I added a convenience function for that.
>
> Thanks,
> Richard
>
>
> gcc/
> * recog.h (recog_op_alt): Convert to a flat array.
> (which_op_alt): New function.
> * recog.c (recog_op_alt): Convert to a flat array.
> (preprocess_constraints): Update accordingly, grouping all
> operands of the same alternative together, rather than the
> other way around.
> * ira-lives.c (check_and_make_def_conflict): Likewise.
> (make_early_clobber_and_input_conflicts): Likewise.
> * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> * reg-stack.c (check_asm_stack_operands): Use which_op_alt.
> (subst_asm_stack_regs): Likewise.
> * regcprop.c (copyprop_hardreg_forward_1): Likewise.
> * regrename.c (hide_operands, record_out_operands): Likewise.
> (build_def_use): Likewise.
> * sel-sched.c (get_reg_class): Likewise.
> * config/arm/arm.c (note_invalid_constants): Likewise.
Largely mechanical. Looks good to me.
Jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints
2014-05-31 9:09 ` [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints Richard Sandiford
@ 2014-06-03 21:53 ` Jeff Law
0 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2014-06-03 21:53 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:09, Richard Sandiford wrote:
> Since the aim of this series is to cache the result of preprocess_constraints,
> we need to make sure that passes don't modify the information afterwards.
> This patch deals with the places that did. Patch 4 will make the information
> properly const.
>
> Thanks,
> Richard
>
>
> gcc/
> * recog.h (alternative_class): New function.
> (which_op_alt): Return a const recog_op_alt.
> * reg-stack.c (check_asm_stack_operands): Update type accordingly.
> (subst_asm_stack_regs): Likewise.
> * config/arm/arm.c (note_invalid_constants): Likewise.
> * regcprop.c (copyprop_hardreg_forward_1): Likewise. Don't modify
> the operand_alternative; use alternative class instead.
> * sel-sched.c (get_reg_class): Likewise.
> * regrename.c (build_def_use): Likewise.
> (hide_operands, restore_operands, record_out_operands): Update type
> accordingly.
OK.
Jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute
2014-05-31 9:15 ` [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute Richard Sandiford
@ 2014-06-03 21:58 ` Jeff Law
2014-06-04 17:37 ` Richard Sandiford
0 siblings, 1 reply; 20+ messages in thread
From: Jeff Law @ 2014-06-03 21:58 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:15, Richard Sandiford wrote:
> As described in the covering note, it seems better to put the onus of
> checking the enabled attribute on the passes that are walking each
> alternative, like LRA does for its internal subpasses. That leads
> to a nicer interface in patch 4 and would make it easier to precompute
> the information at build time. (The only thing preventing that now is
> the subunion class.)
>
> Thanks,
> Richard
>
>
> gcc/
> * recog.c (preprocess_constraints): Don't skip disabled alternatives.
> * ira-lives.c (check_and_make_def_conflict): Check for disabled
> alternatives.
> (make_early_clobber_and_input_conflicts): Likewise.
> * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
Did you spot check the other ports which utilized the enabled attribute
to see if they need tweaking too?
I see aarch64, alpha, arc, arm, avr, c6x m68k, mips, mn10300, nds32,
s390, sh & sparc. I didn't check to see if any of them walk the
alternatives in the backend.
Assuming you at least spot checked them, then this patch is OK.
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/5] Cache recog_op_alt by insn code: main patch
2014-05-31 9:17 ` [PATCH 4/5] Cache recog_op_alt by insn code: main patch Richard Sandiford
@ 2014-06-03 22:01 ` Jeff Law
0 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2014-06-03 22:01 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:17, Richard Sandiford wrote:
> This is the refreshed main patch. I've rearranged the preprocess_constraints
> code into three functions so that LRA can use it in patch 5.
>
> Thanks,
> Richard
>
>
> gcc/
> * recog.h (operand_alternative): Convert reg_class, reject,
> matched and matches into bitfields.
> (preprocess_constraints): New overload.
> (preprocess_insn_constraints): New function.
> (preprocess_constraints): Take the insn as parameter.
> (recog_op_alt): Change into a pointer.
> (target_recog): Add x_op_alt.
> * recog.c (asm_op_alt): New variable.
> (recog_op_alt): Change into a pointer.
> (preprocess_constraints): New overload, replacing the old function
> definition with one that doesn't use global state.
> (preprocess_insn_constraints): New function.
> (preprocess_constraints): Use them. Take the insn as parameter.
> Use asm_op_alt for asms.
> (recog_init): Free existing x_op_alt entries.
> * ira-lives.c (check_and_make_def_conflict): Make operand_alternative
> pointer const.
> (make_early_clobber_and_input_conflicts): Likewise.
> (process_bb_node_lives): Pass the insn to process_constraints.
> * reg-stack.c (check_asm_stack_operands): Likewise.
> (subst_asm_stack_regs): Likewise.
> * regcprop.c (copyprop_hardreg_forward_1): Likewise.
> * regrename.c (build_def_use): Likewise.
> * sched-deps.c (sched_analyze_insn): Likewise.
> * sel-sched.c (get_reg_class, implicit_clobber_conflict_p): Likewise.
> * config/arm/arm.c (xscale_sched_adjust_cost): Likewise.
> (note_invalid_constants): Likewise.
> * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> (ix86_legitimate_combined_insn): Make operand_alternative pointer
> const.
OK.
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/5] Reuse recog_op_alt cache in LRA
2014-05-31 9:23 ` [PATCH 5/5] Reuse recog_op_alt cache in LRA Richard Sandiford
@ 2014-06-03 22:02 ` Jeff Law
0 siblings, 0 replies; 20+ messages in thread
From: Jeff Law @ 2014-06-03 22:02 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On 05/31/14 03:22, Richard Sandiford wrote:
> The operand_alternative cache was LRA's only per-subtarget state
> so a lot of this patch is removing the associated initialisation
> and target_globals stuff.
>
> I called the preprocess_constraints functions in lra_set_insn_recog_data
> rather than setup_operand_alternative because lra_set_insn_recog_data
> already has a local constraints array (needed for asms).
> setup_operand_alternative is now only called in the uncached case.
>
> Thanks,
> Richard
>
>
> gcc/
> * lra-int.h (lra_static_insn_data): Make operand_alternative a
> const pointer.
> (target_lra_int, default_target_lra_int, this_target_lra_int)
> (op_alt_data): Delete.
> * lra.h (lra_init): Delete.
> * lra.c (default_target_lra_int, this_target_lra_int): Delete.
> (init_insn_code_data_once): Remove op_alt_data handling.
> (finish_insn_code_data_once): Likewise.
> (init_op_alt_data): Delete.
> (get_static_insn_data): Initialize operand_alternative to null.
> (free_insn_recog_data): Cast operand_alternative before freeing it.
> (setup_operand_alternative): Take the operand_alternative as
> parameter and assume it isn't already cached in the static
> insn data.
> (lra_set_insn_recog_data): Update accordingly.
> (lra_init): Delete.
> * ira.c (ira_init): Don't call lra_init.
> * target-globals.h (this_target_lra_int): Declare.
> (target_globals): Remove lra_int.
> (restore_target_globals): Update accordingly.
> * target-globals.c: Don't include lra-int.h.
> (default_target_globals, save_target_globals): Remove lra_int.
Also good for the trunk. Got to love patches that delete big blobs of
code :-)
jeff
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute
2014-06-03 21:58 ` Jeff Law
@ 2014-06-04 17:37 ` Richard Sandiford
0 siblings, 0 replies; 20+ messages in thread
From: Richard Sandiford @ 2014-06-04 17:37 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Thanks for the reviews, now committed
Jeff Law <law@redhat.com> writes:
> On 05/31/14 03:15, Richard Sandiford wrote:
>> As described in the covering note, it seems better to put the onus of
>> checking the enabled attribute on the passes that are walking each
>> alternative, like LRA does for its internal subpasses. That leads
>> to a nicer interface in patch 4 and would make it easier to precompute
>> the information at build time. (The only thing preventing that now is
>> the subunion class.)
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>> * recog.c (preprocess_constraints): Don't skip disabled alternatives.
>> * ira-lives.c (check_and_make_def_conflict): Check for disabled
>> alternatives.
>> (make_early_clobber_and_input_conflicts): Likewise.
>> * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise.
> Did you spot check the other ports which utilized the enabled attribute
> to see if they need tweaking too?
>
> I see aarch64, alpha, arc, arm, avr, c6x m68k, mips, mn10300, nds32,
> s390, sh & sparc. I didn't check to see if any of them walk the
> alternatives in the backend.
Yeah, the only port besides i386 to use preprocess_constraints is arm,
but it only looks at alternative which_alternative.
Richard
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/5] Cache recog_op_alt by insn code, take 2
2014-06-03 21:38 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Jeff Law
@ 2014-06-04 17:39 ` Richard Sandiford
0 siblings, 0 replies; 20+ messages in thread
From: Richard Sandiford @ 2014-06-04 17:39 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Jeff Law <law@redhat.com> writes:
> On 05/31/14 03:02, Richard Sandiford wrote:
>> A second difference was that preprocess_constraints skips disabled
>> alternatives while LRA's setup_operand_alternative doesn't; LRA just
>> checks for disabled alternatives when walking the array instead.
>> That should make no difference in practice, but since the LRA approach
>> would also make it easier to precompute the array at build time,
>> I thought it would be a better way to go. It also gives a cleaner
>> interface: we can have a preprocess_insn_constraints function that
>> takes a (define_insn-based) insn and returns the operand_alternative
>> information without any global state.
> Sounds good. Do you think prebuilding those arrays once at build time
> is likely to have a measurable impact? I realize you probably haven't
> done measurements, just looking for your gut instinct here.
I have a local patch that does it, but unfortunately it doesn't seem to
make a measurable difference. I'll try measuring it again once other
lower-hanging fruit are out of the way.
>> Also, I hadn't realised that a define_insn with no constraints
>> at all has 0 alternatives rather than 1. Some passes nevertheless
>> unconditionally access the recog_op_alt information for alternative
>> which_alternative.
>>
>> I could have fixed that by making n_alternatives always be >= 1
>> in the static insn table. Several places do have fast paths for
>> n_alternatives == 0 though, so I thought it would be better to
>> handle the 0 alternative case in preprocess_constraints as well.
>> All it needs is a MIN (..., 1).
> Funny thing is I suspect the n_alternatives == 0 case is rare in
> practice though.
Yeah, probably. :-)
Thanks,
Richard
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-06-04 17:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-20 8:19 RFA: cache recog_op_alt by insn code Richard Sandiford
2014-05-20 18:04 ` Jeff Law
2014-05-26 19:21 ` Require '%' to be at the beginning of a constraint string Richard Sandiford
2014-05-27 17:41 ` Jeff Law
2014-05-28 19:50 ` Richard Sandiford
2014-05-30 16:24 ` Jeff Law
2014-05-31 9:02 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Richard Sandiford
2014-05-31 9:06 ` [PATCH 1/5] Flatten recog_op_alt and reorder entries Richard Sandiford
2014-06-03 21:50 ` Jeff Law
2014-05-31 9:09 ` [PATCH 2/5] Don't modify recog_op_alt after preprocess_constraints Richard Sandiford
2014-06-03 21:53 ` Jeff Law
2014-05-31 9:15 ` [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute Richard Sandiford
2014-06-03 21:58 ` Jeff Law
2014-06-04 17:37 ` Richard Sandiford
2014-05-31 9:17 ` [PATCH 4/5] Cache recog_op_alt by insn code: main patch Richard Sandiford
2014-06-03 22:01 ` Jeff Law
2014-05-31 9:23 ` [PATCH 5/5] Reuse recog_op_alt cache in LRA Richard Sandiford
2014-06-03 22:02 ` Jeff Law
2014-06-03 21:38 ` [PATCH 0/5] Cache recog_op_alt by insn code, take 2 Jeff Law
2014-06-04 17:39 ` Richard Sandiford
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).