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: Add static size report when using -fstack-usage for ARM targets


Hi Thomas,

Thanks for attempting this but there are a few comments that I had
inspite of the fact that I can't accept or reject your patch.

On Sun, Nov 21, 2010 at 2:37 PM, Thomas Klein <th.r.klein@web.de> wrote:
> Hello
>
> With GCC 4.6 a new switch -fstack-usage has been added.
> Some target architectures have support for this.
> To give ARM targets support for this feature only a few lines of code are
> missing.
> Is it possible to add this or something similar?

Patches should be submitted to gcc-patches@gcc.gnu.org . Please take
all follow-ups there. I cannot approve or reject your patch but
noticed the following bits in your patch.

Finally all patches must be tested on the appropriate platform.  Other
comments are inline below.

>
>
> 2010-11-21 ?Thomas Klein <th.r.klein@web.de>
>
> ? ?* config/arm/arm.c (arm_expand_prologue): Report the static ?..
> ? ?* config/arm/arm.c (thumb1_expand_prologue): .. stack size if
> -fstack-usage is used.

Might be better to have 1 proper entry for arm_expand_prologue and
Likewise for the thumb1_expand_prologue change log entry.

Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c ? ? ? ?(revision 167002)
> +++ gcc/config/arm/arm.c ? ? ? ?(working copy)
> @@ -15722,6 +15722,13 @@ arm_expand_prologue (void)
> ? ? ? ?}
> ? ? }
>
> + ?if (flag_stack_usage)
> + ? ?{
> + ? ? ?HOST_WIDE_INT stack_size = saved_regs;
> + ? ? ?current_function_static_stack_size = stack_size;
> + ? ?}

Why do you need a new local variable here ?

> +
> +

1 new line is enough here.


> ? if (offsets->outgoing_args != offsets->saved_args + saved_regs)
> ? ? {
> ? ? ? /* This add can produce multiple insns for a large constant, so we
> @@ -15733,6 +15740,12 @@ arm_expand_prologue (void)
>
> ? ? ? insn = emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?amount));
> + ? ? ?if (flag_stack_usage)
> + ? ? ? ?{
> + ? ? ? ? ? HOST_WIDE_INT stack_size = offsets->outgoing_args -
> (offsets->saved_args + saved_regs);
> + ? ? ? ? ? current_function_static_stack_size += stack_size;
> + ? ? ? ?}
> +

I don't think there's any need to have another local variable here.
Can't you just calculate the value in another variable and use the
same value here as well as in the initialization of amount ?

> ? ? ? do
> ? ? ? ?{
> ? ? ? ? ?last = last ? NEXT_INSN (last) : get_insns ();
> @@ -20535,6 +20548,10 @@ thumb1_expand_prologue (void)
> ? ? ? ? ? ? ? ? ? ?stack_pointer_rtx);
>
> ? amount = offsets->outgoing_args - offsets->saved_regs;
> + ?if (flag_stack_usage)
> + ? ?{
> + ? ? ? current_function_static_stack_size = amount;
> + ? ?}
> ? amount -= 4 * thumb1_extra_regs_pushed (offsets, true);
> ? if (amount)
> ? ? {
> + ? ? ?current_function_static_stack_size = stack_size;
> + ? ?}
> +
> +

Again an extra new line

> ? if (offsets->outgoing_args != offsets->saved_args + saved_regs)
> ? ? {
> ? ? ? /* This add can produce multiple insns for a large constant, so we
> @@ -15733,6 +15740,12 @@ arm_expand_prologue (void)
>
> ? ? ? insn = emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?amount));
> + ? ? ?if (flag_stack_usage)
> + ? ? ? ?{
> + ? ? ? ? ? HOST_WIDE_INT stack_size = offsets->outgoing_args -
> (offsets->saved_args + saved_regs);
> + ? ? ? ? ? current_function_static_stack_size += stack_size;
> + ? ? ? ?}
> +
> ? ? ? do
> ? ? ? ?{
> ? ? ? ? ?last = last ? NEXT_INSN (last) : get_insns ();
> @@ -20535,6 +20548,10 @@ thumb1_expand_prologue (void)
> ? ? ? ? ? ? ? ? ? ?stack_pointer_rtx);
>
> ? amount = offsets->outgoing_args - offsets->saved_regs;
> + ?if (flag_stack_usage)
> + ? ?{
> + ? ? ? current_function_static_stack_size = amount;
> + ? ?}
> ? amount -= 4 * thumb1_extra_regs_pushed (offsets, true);

Surely, the stack size has to be adjusted after the value of amount
has been adjusted unless I misunderstand this ?


cheers
Ramana

---
Ramana Radhakrishnan


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