Ping
On 12/11/2019 09:11, Matthew Malcomson wrote:
> In scheduling passes, notes are removed with `remove_notes` before the
> scheduling is done, and added back in with `reemit_notes` once the
> scheduling has been decided.
>
> This process leaves the notes in the RTL chain with different insn uid's
> than were there before. Having different UID's (larger than the
> previous ones) means that DF_INSN_INFO_GET(insn) will access outside of
> the allocated array.
>
> This has been seen in the `regstat_bb_compute_calls_crossed` function.
> This patch adds an assert to the `regstat_bb_compute_calls_crossed`
> function so that bad accesses here are caught instead of going
> unnoticed, and then avoids the problem.
>
> We avoid the problem by ensuring that new notes added by `reemit_notes` have
> an
> insn record given to them. This is done by adding a call to
> `df_insn_create_insn_record` on each note added in `reemit_notes`.
> `df_insn_create_insn_record` leaves this new record zeroed out, which appears
> to be fine for notes (e.g. `df_bb_refs_record` already does not set
> anything except the luid for notes, and notes have no dataflow information to
> record).
>
> We add the testcase that Martin found here
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410#c2 .
> This testcase fails with the "regstat.c" change, and then succeeds with the
> "haifa-sched.c" change.
>
> There is a similar problem with labels, that the `gcc_assert` catches
> when running regression tests in gcc.dg/fold-eqandshift-1.c and
> gcc.c-torture/compile/pr32482.c.
> This is due to the `cfg_layout_finalize` call in `bb-reorder.c` emitting
> new labels for the start of the newly created basic blocks. These labels are
> not given a dataflow df_insn_info member.
>
> We solve this by manually calling `df_recompute_luids` on each basic
> block once this pass has finished.
>
> Testing done:
> Bootstrapped and regression test on aarch64-none-linux-gnu native.
>
> gcc/ChangeLog:
>
> 2019-11-12 Matthew Malcomson <matthew.malcom...@arm.com>
>
> PR middle-end/92410
> * bb-reorder.c (pass_reorder_blocks::execute): Recompute
> dataflow luids once basic blocks have been reordered.
> * haifa-sched.c (reemit_notes): Create df insn record for each
> new note.
> * regstat.c (regstat_bb_compute_calls_crossed): Assert every
> insn has an insn record before trying to use it.
>
> gcc/testsuite/ChangeLog:
>
> 2019-11-12 Matthew Malcomson <matthew.malcom...@arm.com>
>
> PR middle-end/92410
> * gcc.dg/torture/pr92410.c: New test.
>
>
>
> ############### Attachment also inlined for ease of reply
> ###############
>
>
> diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
> index
> 0ac39140c6ce3db8499f99cd8f483218888de61b..4943f97a2cfccc7ab7c4834fff2c81f62fc5b738
> 100644
> --- a/gcc/bb-reorder.c
> +++ b/gcc/bb-reorder.c
> @@ -2662,6 +2662,8 @@ pass_reorder_blocks::execute (function *fun)
> bb->aux = bb->next_bb;
> cfg_layout_finalize ();
>
> + FOR_EACH_BB_FN (bb, fun)
> + df_recompute_luids (bb);
> return 0;
> }
>
> diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
> index
> 41cf1f362e8c34d009b0a310ff5b9a9ffb613631..2e1a84f1325d54ece1cf3c6f0bab607099c71d27
> 100644
> --- a/gcc/haifa-sched.c
> +++ b/gcc/haifa-sched.c
> @@ -5433,6 +5433,7 @@ reemit_notes (rtx_insn *insn)
>
> last = emit_note_before (note_type, last);
> remove_note (insn, note);
> + df_insn_create_insn_record (last);
> }
> }
> }
> diff --git a/gcc/regstat.c b/gcc/regstat.c
> index
> 4da9b7cc523cde113df931226ea56310b659e619..c6cefb117d7330cc1b9787c37083e05e2acda2d2
> 100644
> --- a/gcc/regstat.c
> +++ b/gcc/regstat.c
> @@ -324,6 +324,7 @@ regstat_bb_compute_calls_crossed (unsigned int bb_index,
> bitmap live)
>
> FOR_BB_INSNS_REVERSE (bb, insn)
> {
> + gcc_assert (INSN_UID (insn) < DF_INSN_SIZE ());
> struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn);
> unsigned int regno;
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr92410.c
> b/gcc/testsuite/gcc.dg/torture/pr92410.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..628e329782260ef36f26506bfbc0d36a93b33f41
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr92410.c
> @@ -0,0 +1,8 @@
> +/* PR middle-end/92410 */
> +/* { dg-do compile } */
> +int v;
> +
> +int a() {
> + ;
> + return v;
> +}
>
