RFA: cache recog_op_alt by insn code

[Jeff Law]

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
>