RFA: cache enabled attribute by insn code

Jeff Law law@redhat.com
Tue May 20 17:50:00 GMT 2014


On 05/20/14 02:16, Richard Sandiford wrote:
> get_attr_enabled was showing up high in a -O0 compile of fold-const.ii.
> At the moment, extract_insn calls this function for every alternative
> on each extraction, which can be expensive for instructions like
> moves that have many alternatives.
>
> The attribute is only supposed to depend on the insn code and the
> current target.  It isn't allowed to depend on the values of operands.
> LRA already makes use of this to cache the enabled attributes based on code.
>
> This patch makes the caching compiler-wide.  It also converts the bool
> array into a plain int bitfield, since at the moment we don't allow more
> than 30 alternatives.  (It's possible we might want to increase it
> beyond 32 at some point, but then we can change the type to uint64_t
> instead.  Wanting to go beyond 64 would suggest deeper problems
> somewhere. :-))
>
> The patch gives a consistent compile-time improvement of about ~3.5%
> on the -O0 fold-const.ii testcase.
>
> There were a few instances of the construct:
>
>    cl = NO_REGS;
>    for (ignore_p = false, curr_alt = 0;
>         (c = *constraints);
>         constraints += CONSTRAINT_LEN (c, constraints))
>      if (c == '#' || !recog_data.alternative_enabled_p[curr_alt])
>        ignore_p = true;
>      else if (c == ',')
>        {
> 	curr_alt++;
> 	ignore_p = false;
>        }
>      else if (! ignore_p)
>
> This had the effect of ignoring all alternatives after the first
> attribute-disabled one, since once we found a disabled alternative we'd
> always enter the first arm of the "if" and never increment curr_alt.
> I don't think that was intentional.
>
> Tested on x86_64-linux-gnu.  OK to install?
>
> I notice ira_setup_alts is using a HARD_REG_SET to store a mask
> of alternatives.  If this patch is OK, I'll follow up with a patch
> to use alternative_mask there too.
>
> Thanks,
> Richard
>
>
> gcc/
> 	* system.h (TEST_BIT): New macro.
> 	* recog.h (alternative_mask): New type.
> 	(ALL_ALTERNATIVES, ALTERNATIVE_BIT): New macros.
> 	(recog_data_d): Replace alternative_enabled_p array with
> 	enabled_alternatives.
> 	(target_recog): New structure.
> 	(default_target_recog, this_target_recog): Declare.
> 	(get_enabled_alternatives): Likewise.
> 	* recog.c (default_target_recog, this_target_recog): New variables.
> 	(get_enabled_alternatives): New function.
> 	(extract_insn): Use it.
> 	(preprocess_constraints, constrain_operands): Adjust for change to
> 	recog_data.
> 	* postreload.c (reload_cse_simplify_operands): Likewise.
> 	* reload.c (find_reloads): Likewise.
> 	* ira-costs.c (record_reg_classes): Likewise.
> 	* ira-lives.c (single_reg_class): Likewise.  Fix bug in which
> 	all alternatives after a disabled one would be skipped.
> 	(ira_implicitly_set_insn_hard_regs): Likewise.
> 	* ira.c (commutative_constraint_p): Likewise.
> 	(ira_setup_alts): Adjust for change to recog_data.
> 	* lra-int.h (lra_insn_recog_data): Replace alternative_enabled_p
> 	with enabled_alternatives.
> 	* lra.c (free_insn_recog_data): Update accordingly.
> 	(lra_update_insn_recog_data): Likewise.
> 	(lra_set_insn_recog_data): Likewise.  Use get_enabled_alternatives.
> 	* lra-constraints.c (process_alt_operands): Likewise.  Handle
> 	only_alternative as part of the enabled mask.
> 	* target-globals.h (this_target_recog): Declare.
> 	(target_globals): Add a recog field.
> 	(restore_target_globals): Restore this_target_recog.
> 	* target-globals.c: Include recog.h.
> 	(default_target_globals): Initialize recog field.
> 	(save_target_globals): Likewise.
This is OK for the trunk (referring to the follow-up message which fixed 
EWRONGPATCH.


jeff



More information about the Gcc-patches mailing list