This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA: cache enabled attribute by insn code


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]