Re: genattrtab speedup 2/4

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Aug 1, 2005 at 12:12 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> this is the second patch, which removes the 'optimize_attrs' function from
> genattrtab. ?This is the source of the biggest speedup in building and
> compiling insn-attrtab.c. ?It can result in overall compilation being a
> bit slower, as more function calls are done at run time, the next two
> patches will give back the performance.
>
> This also fixes a real bug. ?write_attr_set is able to take an insn_code
> associated with the value to output, and uses that to do some more
> optimizations of the value based on that insn_code. ?The calls to
> write_attr_set() simply used av->first_insn->def->insn_code for that. ?But
> if a value is associated with more than one insn (i.e. if av->first_insn
> is not the only one), then there will be misoptimizations applied, because
> write_attr_set() and subfunctions will optimize the value targeting for
> that first insn, which of course is incorrect when interpreting it in the
> context of the second insn. ?One needs to test for av->num_insns being 1
> before being able to call write_attr_set() with a certain insn_code,
> otherwiese -2 must be used.
>
> This bug is hidden practically as long as optimize_attrs runs before,
> because it optimizes all values per insn already and only collects those
> together which really come out the same for all insns. ?(this also means
> the optimization in write_attr_set is not very effective if optimize_attrs
> runs). ?Removing it exposes this bug.
>
> write_attr_set() iterates over the individual conditions in a value,
> collecting together which facts are know true already, in order to
> optimize following conditions. ?Without optimize_attrs() this chain can
> become extremely long for values associated with many insns, makeing it
> very slow. ?I put in a stop gap for that.
>
> Without optimize_attr we can make use of optimizing at least the given
> value before actually writing it in write_attr_case. ?There are many
> instances where this results in simplification, especially when a certain
> attribute is not using the general fallback definition, but one specific
> for that insn.

Ok if the patch still bootstraps/regtests.

Thanks,
Richard.

>
> Ciao,
> Michael.
> --
> ? ? ? ?* genattrtab.c (optimize_attrs): Remove.
> ? ? ? ?(write_attr_get): Use insn_code member only when only one insn
> ? ? ? ?is associated with the value.
> ? ? ? ?(write_attr_set): Don't let our_known_true become too long.
> ? ? ? ?(write_attr_case): Optimize value.
> ? ? ? ?(main): Don't call optimize_attrs.
>
> --- genattrtab.clean.c ?2005-07-29 19:48:05.000000000 +0200
> +++ genattrtab.noopt.c ?2005-07-31 22:53:01.557627043 +0200
> @@ -88,7 +88,7 @@ Software Foundation, 51 Franklin Street,
> ? ?`unchanging' (ATTR_IND_SIMPLIFIED_P): This rtx is fully simplified
> ? ? ? independent of the insn code.
> ? ?`in_struct' (ATTR_CURR_SIMPLIFIED_P): This rtx is fully simplified
> - ? ? ?for the insn code currently being processed (see optimize_attrs).
> + ? ? ?for the insn code currently being processed.
> ? ?`return_val' (ATTR_PERMANENT_P): This rtx is permanent and unique
> ? ? ? (see attr_rtx). ?*/
>
> @@ -298,7 +298,6 @@ static rtx simplify_and_tree ? ? ? ?(rtx, rtx *
> ?static rtx simplify_or_tree ? ?(rtx, rtx *, int, int);
> ?static rtx simplify_test_exp ? (rtx, int, int);
> ?static rtx simplify_test_exp_in_temp (rtx, int, int);
> -static void optimize_attrs ? ? (void);
> ?static void gen_attr ? ? ? ? ? (rtx, int);
> ?static int count_alternatives ?(rtx);
> ?static int compares_alternatives_p (rtx);
> @@ -2794,105 +2793,6 @@ simplify_test_exp (rtx exp, int insn_cod
> ? return newexp;
> ?}
>
> -/* Optimize the attribute lists by seeing if we can determine conditional
> - ? values from the known values of other attributes. ?This will save subroutine
> - ? calls during the compilation. ?*/
> -
> -static void
> -optimize_attrs (void)
> -{
> - ?struct attr_desc *attr;
> - ?struct attr_value *av;
> - ?struct insn_ent *ie;
> - ?rtx newexp;
> - ?int i;
> - ?struct attr_value_list
> - ?{
> - ? ?struct attr_value *av;
> - ? ?struct insn_ent *ie;
> - ? ?struct attr_desc *attr;
> - ? ?struct attr_value_list *next;
> - ?};
> - ?struct attr_value_list **insn_code_values;
> - ?struct attr_value_list *ivbuf;
> - ?struct attr_value_list *iv;
> -
> - ?/* For each insn code, make a list of all the insn_ent's for it,
> - ? ? for all values for all attributes. ?*/
> -
> - ?if (num_insn_ents == 0)
> - ? ?return;
> -
> - ?/* Make 2 extra elements, for "code" values -2 and -1. ?*/
> - ?insn_code_values = xcalloc ((insn_code_number + 2),
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? sizeof (struct attr_value_list *));
> -
> - ?/* Offset the table address so we can index by -2 or -1. ?*/
> - ?insn_code_values += 2;
> -
> - ?iv = ivbuf = xmalloc (num_insn_ents * sizeof (struct attr_value_list));
> -
> - ?for (i = 0; i < MAX_ATTRS_INDEX; i++)
> - ? ?for (attr = attrs[i]; attr; attr = attr->next)
> - ? ? ?for (av = attr->first_value; av; av = av->next)
> - ? ? ? for (ie = av->first_insn; ie; ie = ie->next)
> - ? ? ? ? {
> - ? ? ? ? ? iv->attr = attr;
> - ? ? ? ? ? iv->av = av;
> - ? ? ? ? ? iv->ie = ie;
> - ? ? ? ? ? iv->next = insn_code_values[ie->def->insn_code];
> - ? ? ? ? ? insn_code_values[ie->def->insn_code] = iv;
> - ? ? ? ? ? iv++;
> - ? ? ? ? }
> -
> - ?/* Sanity check on num_insn_ents. ?*/
> - ?gcc_assert (iv == ivbuf + num_insn_ents);
> -
> - ?/* Process one insn code at a time. ?*/
> - ?for (i = -2; i < insn_code_number; i++)
> - ? ?{
> - ? ? ?/* Clear the ATTR_CURR_SIMPLIFIED_P flag everywhere relevant.
> - ? ? ? ?We use it to mean "already simplified for this insn". ?*/
> - ? ? ?for (iv = insn_code_values[i]; iv; iv = iv->next)
> - ? ? ? clear_struct_flag (iv->av->value);
> -
> - ? ? ?for (iv = insn_code_values[i]; iv; iv = iv->next)
> - ? ? ? {
> - ? ? ? ? struct obstack *old = rtl_obstack;
> -
> - ? ? ? ? attr = iv->attr;
> - ? ? ? ? av = iv->av;
> - ? ? ? ? ie = iv->ie;
> - ? ? ? ? if (GET_CODE (av->value) != COND)
> - ? ? ? ? ? continue;
> -
> - ? ? ? ? rtl_obstack = temp_obstack;
> - ? ? ? ? newexp = av->value;
> - ? ? ? ? while (GET_CODE (newexp) == COND)
> - ? ? ? ? ? {
> - ? ? ? ? ? ? rtx newexp2 = simplify_cond (newexp, ie->def->insn_code,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ie->def->insn_index);
> - ? ? ? ? ? ? if (newexp2 == newexp)
> - ? ? ? ? ? ? ? break;
> - ? ? ? ? ? ? newexp = newexp2;
> - ? ? ? ? ? }
> -
> - ? ? ? ? rtl_obstack = old;
> - ? ? ? ? if (newexp != av->value)
> - ? ? ? ? ? {
> - ? ? ? ? ? ? newexp = attr_copy_rtx (newexp);
> - ? ? ? ? ? ? remove_insn_ent (av, ie);
> - ? ? ? ? ? ? av = get_attr_value (newexp, attr, ie->def->insn_code);
> - ? ? ? ? ? ? iv->av = av;
> - ? ? ? ? ? ? insert_insn_ent (av, ie);
> - ? ? ? ? ? }
> - ? ? ? }
> - ? ?}
> -
> - ?free (ivbuf);
> - ?free (insn_code_values - 2);
> -}
> -
> ?/* Clear the ATTR_CURR_SIMPLIFIED_P flag in EXP and its subexpressions. ?*/
>
> ?static void
> @@ -3677,10 +3577,13 @@ write_attr_get (struct attr_desc *attr)
> ? ? ? printf ("{\n");
>
> ? ? ? for (av = attr->first_value; av; av = av->next)
> - ? ? ? if (av->num_insns != 0)
> + ? ? ? if (av->num_insns == 1)
> ? ? ? ? ?write_attr_set (attr, 2, av->value, "return", ";",
> ? ? ? ? ? ? ? ? ? ? ? ? ?true_rtx, av->first_insn->def->insn_code,
> ? ? ? ? ? ? ? ? ? ? ? ? ?av->first_insn->def->insn_index);
> + ? ? ? else if (av->num_insns != 0)
> + ? ? ? ? write_attr_set (attr, 2, av->value, "return", ";",
> + ? ? ? ? ? ? ? ? ? ? ? ? true_rtx, -2, 0);
>
> ? ? ? printf ("}\n\n");
> ? ? ? return;
> @@ -3751,6 +3654,8 @@ write_attr_set (struct attr_desc *attr,
> ? ? ? ? ?rtx testexp;
> ? ? ? ? ?rtx inner_true;
>
> + ? ? ? ? if (insn_code == -2)
> + ? ? ? ? ? our_known_true = known_true; /* Only reset it after some time. ?*/
> ? ? ? ? ?testexp = eliminate_known_true (our_known_true,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?XVECEXP (value, 0, i),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?insn_code, insn_index);
> @@ -3844,6 +3749,7 @@ write_attr_case (struct attr_desc *attr,
> ? ? ? ? ? ? ? ? int write_case_lines, const char *prefix, const char *suffix,
> ? ? ? ? ? ? ? ? int indent, rtx known_true)
> ?{
> + ?rtx opt_val;
> ? if (av->num_insns == 0)
> ? ? return;
>
> @@ -3882,9 +3788,26 @@ write_attr_case (struct attr_desc *attr,
> ? ? ? printf ("extract_insn_cached (insn);\n");
> ? ? }
>
> - ?write_attr_set (attr, indent + 2, av->value, prefix, suffix,
> - ? ? ? ? ? ? ? ? known_true, av->first_insn->def->insn_code,
> - ? ? ? ? ? ? ? ? av->first_insn->def->insn_index);
> + ?opt_val = av->value;
> + ?while (GET_CODE (opt_val) == COND)
> + ? ?{
> + ? ? ?rtx newexp2;
> + ? ? ?if (av->num_insns == 1)
> + ? ? ? newexp2 = simplify_cond (opt_val, av->first_insn->def->insn_code,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?av->first_insn->def->insn_index);
> + ? ? ?else
> + ? ? ? newexp2 = simplify_cond (opt_val, -2, 0);
> + ? ? ?if (newexp2 == opt_val)
> + ? ? ? break;
> + ? ? ?opt_val = newexp2;
> + ? ?}
> + ?if (av->num_insns == 1)
> + ? ?write_attr_set (attr, indent + 2, opt_val, prefix, suffix,
> + ? ? ? ? ? ? ? ? ? known_true, av->first_insn->def->insn_code,
> + ? ? ? ? ? ? ? ? ? av->first_insn->def->insn_index);
> + ?else
> + ? ?write_attr_set (attr, indent + 2, opt_val, prefix, suffix,
> + ? ? ? ? ? ? ? ? ? known_true, -2, 0);
>
> ? if (strncmp (prefix, "return", 6))
> ? ? {
> @@ -4517,9 +4440,6 @@ from the machine description file `md'.
> ? /* Construct extra attributes for `length'. ?*/
> ? make_length_attrs ();
>
> - ?/* Perform any possible optimizations to speed up compilation. ?*/
> - ?optimize_attrs ();
> -
> ? /* Now write out all the `gen_attr_...' routines. ?Do these before the
> ? ? ?special routines so that they get defined before they are used. ?*/
>
>