On Thu, 10 Feb 2022, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, we get -fcompare-debug failure, which is caused by
> cfg_layout_merge_blocks successfully merging two bbs where both bbs
> contained just CODE_LABEL, NOTE_INSN_BASIC_BLOCK and in the -g case both
> some debug insns at the end. cfg_layout_merge_blocks calls
> update_bb_for_insn_chain which for the post-label insns in the second block
> (except for BARRIERs) calls df_insn_change_bb. This function changes
> the bb of the insns and for notes just punts, but for other insns calls
> df_set_bb_dirty. Now the problem is that because there were only debug
> insns and notes in the second block, df_set_bb_dirty is called on both
> only in the -g case and not with -g0. df_set_bb_dirty these days
> sets both the BB_MODIFIED flag and marks the bb as dirty, and the former
> is what 6 spots in cfgcleanup.cc use in code-generation decisions,
> in this case
> may_thread |= (target->flags & BB_MODIFIED) != 0;
> in particular. So, with -g may_thread is true while with -g0 it is not
> and we diverge from that point onwards.
> I've thought about introducing df_set_bb_dirty_nondebug that wouldn't
> set BB_MODIFIED but would mark the bb dirty, but then I went through
> history and found changes like:
> https://gcc.gnu.org/legacy-ml/gcc-patches/2010-10/msg00059.html
> so I've also tried just not calling df_set_bb_dirty for debug insns
> at all and it passed x86_64-linux and i686-linux
> --enable-checking=yes,rtl,extra,df bootstraps/regtests, so perhaps
> that works too.
> Now that I look at it again, if we don't need those from %d to %d messages
> for debug insns in the dump files, another way to fix it would be just to
> change the very first line in the hunk from
> if (!INSN_P (insn))
> to
> if (!DEBUG_INSN_P (insn))
> Though, df_set_bb_dirty_nondebug which will do everything but
> set bb->flags |= BB_MODIFIED is yet another option I can test.
> Perhaps even that PR42889 was solely about those 6 decisions in cfgcleanup
> (at that point it used df_get_bb_dirty) and not about actually the
> recomputation of some of the problems causing different code generations.
OK.
Thanks,
Richard.
> 2022-02-10 Jakub Jelinek <ja...@redhat.com>
>
> PR rtl-optimization/104459
> * df-scan.cc (df_insn_change_bb): Don't call df_set_bb_dirty when
> moving DEBUG_INSNs between bbs.
>
> * gcc.dg/pr104459.c: New test.
>
> --- gcc/df-scan.cc.jj 2022-01-18 00:18:02.720744815 +0100
> +++ gcc/df-scan.cc 2022-02-10 11:10:54.039135547 +0100
> @@ -1769,13 +1769,15 @@ df_insn_change_bb (rtx_insn *insn, basic
> if (!INSN_P (insn))
> return;
>
> - df_set_bb_dirty (new_bb);
> + if (!DEBUG_INSN_P (insn))
> + df_set_bb_dirty (new_bb);
> if (old_bb)
> {
> if (dump_file)
> fprintf (dump_file, " from %d to %d\n",
> old_bb->index, new_bb->index);
> - df_set_bb_dirty (old_bb);
> + if (!DEBUG_INSN_P (insn))
> + df_set_bb_dirty (old_bb);
> }
> else
> if (dump_file)
> --- gcc/testsuite/gcc.dg/pr104459.c.jj 2022-02-10 11:09:38.397181836
> +0100
> +++ gcc/testsuite/gcc.dg/pr104459.c 2022-02-10 11:09:16.049490953 +0100
> @@ -0,0 +1,38 @@
> +/* PR rtl-optimization/104459 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funswitch-loops -fno-tree-dce -fcompare-debug -w" } */
> +
> +void
> +foo (int x, int y)
> +{
> + unsigned int a;
> +
> + for (;;)
> + {
> + short int *p = (short int *) &x;
> + unsigned int q = 0;
> +
> + a /= 2;
> + if (a)
> + {
> + q -= y;
> + while (q)
> + ;
> + }
> +
> + if (x)
> + {
> + for (q = 0; q != 1; q += 2)
> + {
> + unsigned int n;
> +
> + n = *p ? 0 : q;
> + y += n < 1;
> +
> + n = a || *p;
> + if (n % x == 0)
> + y /= x;
> + }
> + }
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
