Re: [google] remove redundant push {lr} for -mthumb (issue4441050)

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

On Tue, Apr 19, 2011 at 11:41 AM, Guozhi Wei <carrot@google.com> wrote:
> Reload pass tries to determine the stack frame, so it needs to check the
> push/pop lr optimization opportunity. One of the criteria is if there is any
> far jump inside the function. Unfortunately at this time gcc can't decide each
> instruction's length and basic block layout, so it can't know the offset of
> a jump. To be conservative it assumes every jump is a far jump. So any jump
> in a function will prevent this push/pop lr optimization.
>
> To enable the push/pop lr optimization in reload pass, I compute the possible
> maximum length of the function body. If the length is not large enough, far
> jump is not necessary, so we can safely do push/pop lr optimization.

What about hot/cold partitioning?  That might cause jumps to different
sections.

Richard.

> Tested on arm qemu with options -march=armv5te -mthumb, without regression.
>
> This patch is for google/main.
>
> 2011-04-19 ?Guozhi Wei ?<carrot@google.com>
>
> ? ? ? ?Google ref 40255.
> ? ? ? ?* gcc/config/arm/arm.c (SHORTEST_FAR_JUMP_LENGTH): New constant.
> ? ? ? ?(estimate_function_length): New function.
> ? ? ? ?(thumb_far_jump_used_p): No far jump is needed in short function.
>
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c ? ? ? ?(revision 172689)
> +++ gcc/config/arm/arm.c ? ? ? ?(working copy)
> @@ -592,7 +592,9 @@ static const struct default_options arm_
> ? arm_preferred_rename_class
>
> ?struct gcc_target targetm = TARGET_INITIALIZER;
> -
> +
> +#define SHORTEST_FAR_JUMP_LENGTH 2040
> +
> ?/* Obstack for minipool constant handling. ?*/
> ?static struct obstack minipool_obstack;
> ?static char * ? ? ? ? minipool_startobj;
> @@ -20298,6 +20300,17 @@ thumb_shiftable_const (unsigned HOST_WID
> ? return 0;
> ?}
>
> +/* Computes the maximum possible function length. */
> +static int
> +estimate_function_length (void)
> +{
> + ?rtx insn;
> + ?int length = 0;
> + ?for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
> + ? ?length += get_attr_length(insn);
> + ?return length;
> +}
> +
> ?/* Returns nonzero if the current function contains,
> ? ?or might contain a far jump. ?*/
> ?static int
> @@ -20316,6 +20329,16 @@ thumb_far_jump_used_p (void)
> ? if (cfun->machine->far_jump_used)
> ? ? return 1;
>
> + ?/* In reload pass we haven't got the exact jump instruction length,
> + ? ? but we can get a reasonable estimation based on the maximum
> + ? ? possible function length. */
> + ?if (!reload_completed)
> + ? ?{
> + ? ? ?int function_length = estimate_function_length();
> + ? ? ?if (function_length < SHORTEST_FAR_JUMP_LENGTH)
> + ? ? ? return 0;
> + ? ?}
> +
> ? /* If this function is not being called from the prologue/epilogue
> ? ? ?generation code then it must be being called from the
> ? ? ?INITIAL_ELIMINATION_OFFSET macro. ?*/
>
> --
> This patch is available for review at http://codereview.appspot.com/4441050
>