[patch 1/3] Sort du_head list and restrict register class by PREFERRED_RENAME_CLASS
Paolo Bonzini
bonzini@gnu.org
Thu Oct 21 09:19:00 GMT 2010
On 10/21/2010 08:50 AM, Yao Qi wrote:
> +/* Get the next LENGTH element in list from START. If length of
> + list from START is less than LENGTH, return the last one. */
Looks like it is "If the length of the list headed from START is _less
than or equal to_ LENGTH, return the last element".
> +static struct du_head *
> +get_tail_du_head (struct du_head *start, int length)
> +{
> + while (length--&& start->next_chain != NULL)
> + {
> + start = start->next_chain;
> + }
Useless braces.
> +
> + return start;
> +}
> +
> +
> +/* Merge the linked list starting from START_NODE. CMP is callback
> + for comparison. */
This comments leaves some questions unanswered: merge with what? What
is returned, and what is left in *START_NODE?
In other words, something like this should do:
/* Merge the first two sequences of CURRENT_LENGTH nodes in the linked
list pointed by START_NODE. Update START_NODE to point to the
merged nodes, and return a pointer to the last merged node. */
> +static struct du_head *
> +merge(struct du_head **start_node, int current_length,
> + int(*cmp)(const struct du_head *, const struct du_head *))
Spaces required after function name and return type ("int (*cmp) (const
etc.").
> +{
> + int can_merge_p, i, left_count, right_count;
> + struct du_head *left, *right;
> + /* Current node of sort result. */
> + struct du_head *current_sorted_node;
> + /* Tail node of sort, used to connect with next piece of list. */
> + struct du_head *current_tail_node;
> +
> + if (*start_node == NULL)
> + return NULL;
> +
> + can_merge_p = 1;
> + left = right = *start_node;
> + right_count = left_count = 0;
> +
> + for (i = 0; i< current_length; i++)
> + {
> + right = right->next_chain;
> + if (right == NULL)
> + {
> + can_merge_p = 0;
Just return NULL right away.
> + break;
> + }
> + }
> +
> + if (!can_merge_p)
> + return NULL;
In sort_du_head:
> + while (1)
> + {
> + last_tail = merge(head, current_length, cmp);
> + if (last_tail != NULL)
> + {
> + do
> + {
> + last_tail = merge (&last_tail->next_chain,
> + current_length, cmp);
> + }
> + while (last_tail != NULL);
> + current_length *= 2;
> + }
> + else
> + break;
> + }
Very nice algorithm!
What about however something even simpler, like:
do
{
segment = head;
segments = 1;
while ((last_tail = merge(segment, current_length, cmp)) != NULL)
{
segment = &last_tail->next_chain;
segments++;
}
current_length *= 2;
}
while (segments > 1);
(Untested).
> +/* Call back function of merge sort. Return true if A is less than
> + B, otherwise return false. */
> +static int
> +merge_sort_callback(const struct du_head *a,
> + const struct du_head *b)
> +{
> + return a->length< b->length;
> +}
Maybe this can be inlined?
> /* Perform register renaming on the current function. */
>
> static unsigned int
> @@ -195,6 +340,8 @@ regrename_optimize (void)
> if (dump_file)
> dump_def_use_chain (all_chains);
>
> + sort_du_head (&all_chains, merge_sort_callback);
> +
> CLEAR_HARD_REG_SET (unavailable);
> /* Don't clobber traceback for noreturn functions. */
> if (frame_pointer_needed)
> @@ -251,8 +398,11 @@ regrename_optimize (void)
> if (DEBUG_INSN_P (tmp->insn))
> continue;
> n_uses++;
> +#ifndef PREFERRED_RENAME_CLASS
> +#define PREFERRED_RENAME_CLASS(CLASS) CLASS
> +#endif
This goes in defaults.h. I'm not sure about the performance
implications of making this a target hook. Also, it looks like
everything except PREFERRED_RENAME_CLASS is actually a separate
improvement to rename-registers that could go in first. So, I would
structure the series like this.:
- patch 1 does everything except adding PREFERRED_RENAME_CLASS
- patch 2 adds PREFERRED_RENAME_CLASS to docs and code
- patch 3 adds the macro to the ARM backend.
> IOR_COMPL_HARD_REG_SET (this_unavailable,
> - reg_class_contents[tmp->cl]);
> + reg_class_contents[PREFERRED_RENAME_CLASS(tmp->cl)]);
> }
>
> if (n_uses< 2)
> static bool
> gate_handle_regrename (void)
> {
> - return (optimize> 0&& (flag_rename_registers));
> + return ((optimize> 0 || optimize_size)&& (flag_rename_registers));
> }
Not needed, optimize_size should always be set. (Sorry for the garbling
of spaces that my mailer did).
Thanks,
Paolo
More information about the Gcc-patches
mailing list