On Mon, Feb 23, 2015 at 11:53:52AM +0100, Georg-Johann Lay wrote:
> This patch fixes PR64331 which produced wrong code because of outdated (too
> many) REG_DEAD notes. These notes are not (re)computed per default, hence
> do the computation by hand each time avr.c:reg_unused_after is called in a
> different pass.
>
> Ok to apply?
>
I was testing a similar patch a while back. Mine had a call to
df_finish_pass after calling df_analyze, and I didn't "cache" the pass
number. I remember running into ICEs
building libgcc after that - from df_verify, IIRC.
Do you know why df_finish_pass isn't needed in this case? AFAICT, all it
appears to do is to clean up some stuff and enable a verification flag.
Regards
Senthil
> Johann
>
>
> gcc/
> PR target/64331
> * config/avr/avr.c (reg_unused_after): Recompute insn notes if the
> pass changed since the last (re)computation.
> * config/avr/avr.h (machine_function.dead_notes_pass_number): New
> component.
>
> gcc/testsuite/
> PR target/64331
> * gcc.target/avr/torture/pr64331.c: New run test.
>
> Index: config/avr/avr.c
> ===================================================================
> --- config/avr/avr.c (revision 220738)
> +++ config/avr/avr.c (working copy)
> @@ -51,6 +51,7 @@
> #include "target-def.h"
> #include "params.h"
> #include "df.h"
> +#include "tree-pass.h" /* for current_pass */
>
> /* Maximal allowed offset for an address in the LD command */
> #define MAX_LD_OFFSET(MODE) (64 - (signed)GET_MODE_SIZE (MODE))
> @@ -7862,8 +7863,22 @@ avr_adjust_insn_length (rtx insn, int le
> int
> reg_unused_after (rtx insn, rtx reg)
> {
> + if (cfun->machine->dead_notes_pass_number
> + != current_pass->static_pass_number)
> + {
> + /* `dead_or_set_p' needs correct REG_DEAD notes, cf. PR64331. Hence
> + recompute them each time we find ourselves in a different pass.
> + `reg_unused_after' is used during final for insn output, and in
> + some earlier passes it is involved in insn length computations. */
> +
> + df_note_add_problem ();
> + df_analyze ();
> +
> + cfun->machine->dead_notes_pass_number =
> current_pass->static_pass_number;
> + }
> +
> return (dead_or_set_p (insn, reg)
> - || (REG_P(reg) && _reg_unused_after (insn, reg)));
> + || (REG_P (reg) && _reg_unused_after (insn, reg)));
> }
>
> /* Return nonzero if REG is not used after INSN.
> Index: config/avr/avr.h
> ===================================================================
> --- config/avr/avr.h (revision 220738)
> +++ config/avr/avr.h (working copy)
> @@ -592,6 +592,10 @@ struct GTY(()) machine_function
> /* 'true' if the above is_foo predicates are sanity-checked to avoid
> multiple diagnose for the same function. */
> int attributes_checked_p;
> +
> + /* The latest static pass number for which REG_DEAD notes have been
> + computed, cf. PR64331. */
> + int dead_notes_pass_number;
> };
>
> /* AVR does not round pushes, but the existence of this macro is
> Index: testsuite/gcc.target/avr/torture/pr64331.c
> ===================================================================
> --- testsuite/gcc.target/avr/torture/pr64331.c (revision 0)
> +++ testsuite/gcc.target/avr/torture/pr64331.c (revision 0)
> @@ -0,0 +1,38 @@
> +/* { dg-do run } */
> +
> +typedef struct
> +{
> + unsigned a, b;
> +} T2;
> +
> +
> +__attribute__((__noinline__, __noclone__))
> +void foo2 (T2 *t, int x)
> +{
> + if (x != t->a)
> + {
> + t->a = x;
> +
> + if (x && x == t->b)
> + t->a = 20;
> + }
> +}
> +
> +
> +T2 t;
> +
> +int main (void)
> +{
> + t.a = 1;
> + t.b = 1234;
> +
> + foo2 (&t, 1234);
> +
> + if (t.a != 20)
> + __builtin_abort();
> +
> + __builtin_exit (0);
> +
> + return 0;
> +}
> +
