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
