diff mbox

LRA: Fix incorrect register spill/reload

Message ID B5E67142681B53468FAF6B7C313565623D34BDCC@KLMAIL01.kl.imgtec.org
State New
Headers show

Commit Message

Robert Suchanek Oct. 31, 2013, 1:23 p.m. UTC
Hello,

When investigating regression with LRA enabled for mips16 I found incorrect spilling and reload of
registers by callee.  In the case, one register was not saved, although used, and another one never
used but saved/restored. 

The issue appears to be in setting registers ever lived and subsequent passes save/restore the wrong
register(s). I have attached a patch below. I presume that the statement terminator was
typed accidentally as I do not see a justification of the df_set_regs_ever_live() function to be outside
the for loop. Or I am wrong?

Regards,
Robert 

    * lra-spills.c (assign_spill_hard_regs): Removed statement terminator after comment.
    Loop body outside the for loop.

Comments

Vladimir Makarov Oct. 31, 2013, 2:40 p.m. UTC | #1
On 10/31/2013 09:23 AM, Robert Suchanek wrote:
> Hello,
>
> When investigating regression with LRA enabled for mips16 I found incorrect spilling and reload of
> registers by callee.  In the case, one register was not saved, although used, and another one never
> used but saved/restored. 
>
> The issue appears to be in setting registers ever lived and subsequent passes save/restore the wrong
> register(s). I have attached a patch below. I presume that the statement terminator was
> typed accidentally as I do not see a justification of the df_set_regs_ever_live() function to be outside
> the for loop. Or I am wrong?
No,  you are not wrong.  It looks a typo for me.  It was not found
earlier as it is used right now only for x86 general reg spills into sse
regs and sse regs are not saved.

Robert, thanks for finding it and informing.  You can commit the patch
into the trunk.

>  
>
>     * lra-spills.c (assign_spill_hard_regs): Removed statement terminator after comment.
>     Loop body outside the for loop.
>
> diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c
> index 7c0c630..e1cf654 100644
> --- a/gcc/lra-spills.c
> +++ b/gcc/lra-spills.c
> @@ -334,8 +334,8 @@ assign_spill_hard_regs (int *pseudo_regnos, int n)
>        for (nr = 0;
>            nr < hard_regno_nregs[hard_regno][lra_reg_info[regno].biggest_mode];
>            nr++)
> -       /* Just loop.  */;
> -      df_set_regs_ever_live (hard_regno + nr, true);
> +       /* Just loop.  */
> +        df_set_regs_ever_live (hard_regno + nr, true);
>      }
>    bitmap_clear (&ok_insn_bitmap);
>    free (reserved_hard_regs);
>
>
diff mbox

Patch

diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c
index 7c0c630..e1cf654 100644
--- a/gcc/lra-spills.c
+++ b/gcc/lra-spills.c
@@ -334,8 +334,8 @@  assign_spill_hard_regs (int *pseudo_regnos, int n)
       for (nr = 0;
           nr < hard_regno_nregs[hard_regno][lra_reg_info[regno].biggest_mode];
           nr++)
-       /* Just loop.  */;
-      df_set_regs_ever_live (hard_regno + nr, true);
+       /* Just loop.  */
+        df_set_regs_ever_live (hard_regno + nr, true);
     }
   bitmap_clear (&ok_insn_bitmap);
   free (reserved_hard_regs);