On Fri, 29 Apr 2022, Jakub Jelinek wrote:
> Hi!
>
> The following testcase fails -fcompare-debug on aarch64-linux. The problem
> is that for the n variable we create a varpool node, then remove it again
> because the var isn't really used, but it keeps being referenced in debug
> stmts/insns with -g. Later during sched1 pass we ask whether the n var
> can be modified through some store to an anchored variable and with -g
> we create a new varpool node for it again just so that we can find its
> ultimate alias target. Even later on we create some cgraph node for the
> loop parallelization, but as there has been an extra varpool node creation
> in between, we get higher node->order with -g than without.
>
> I think we just shouldn't create varpool nodes during RTL optimizations,
> either the vars exist and we have varpool nodes for those, or they are some
> late created variables which will have itself as ultimate alias target
> (in debugging dumps I've done on powerpc64le-linux for this, 828576 out of
> 828580 cases where symtab_node::get (x_decl) returned NULL here it was
> some DECL_IN_CONSTANT_POOL decl so certainly
> symtab_node::get_create (x_decl)->ultimate_alias_target ()->decl == x_decl
> ), or as in this case it is a var referenced only in debug insns and we'd
> better not to create varpool node for it, just:
> gcc.dg/pr105415.c foo n
> gcc.dg/pr105415.c foo n
> gcc.dg/torture/pr41555.c main g_47
> gcc.dg/torture/pr41555.c main g_47
> ).
> So the following patch calls just get instead of get_create and assumes
> that if we'd create a new varpool node, it would have itself as its
> ultimate_alias_target that late and it would be a definition unless
> DECL_EXTERNAL.
>
> Bootstrapped/regtested on aarch64-linux and powerpc64le-linux, ok for trunk?
I think that's reasonable (we indeed shouldn't create a varpool node
here). I do think that we eventually want to retain removed nodes
but mark them so. In fact any debug references will be thrown away
because of this anyway.
So I wonder if we can instead simply do if (!x_node) return 0;?
The question is also why sched does any queries for debug-insns,
does it merely reset them based on the answer? That said,
it would be nice to be able to assert that x_node is not NULL
and catch this in the callers somehow.
Richard.
> 2022-04-29 Jakub Jelinek <ja...@redhat.com>
>
> PR debug/105415
> * alias.cc (compare_base_symbol_refs): Avoid creating new symtab
> nodes, if it doesn't exist, punt if DECL_EXTERNAL, otherwise use
> x_decl instead of x_node->decl.
>
> * gcc.dg/pr105415.c: New test.
>
> --- gcc/alias.cc.jj 2022-02-21 16:51:50.261232505 +0100
> +++ gcc/alias.cc 2022-04-28 11:57:18.940425126 +0200
> @@ -2219,12 +2219,19 @@ compare_base_symbol_refs (const_rtx x_ba
> || (!TREE_STATIC (x_decl) && !TREE_PUBLIC (x_decl)))
> return 0;
>
> - symtab_node *x_node = symtab_node::get_create (x_decl)
> - ->ultimate_alias_target ();
> - /* External variable cannot be in section anchor. */
> - if (!x_node->definition)
> + symtab_node *x_node = symtab_node::get (x_decl);
> + tree x_decl2 = x_decl;
> + if (x_node != NULL)
> + {
> + x_node = x_node->ultimate_alias_target ();
> + /* External variable cannot be in section anchor. */
> + if (!x_node->definition)
> + return 0;
> + x_decl2 = x_node->decl;
> + }
> + else if (DECL_EXTERNAL (x_decl))
> return 0;
> - x_base = XEXP (DECL_RTL (x_node->decl), 0);
> + x_base = XEXP (DECL_RTL (x_decl2), 0);
> /* If not in anchor, we can disambiguate. */
> if (!SYMBOL_REF_HAS_BLOCK_INFO_P (x_base))
> return 0;
> --- gcc/testsuite/gcc.dg/pr105415.c.jj 2022-04-28 12:26:13.174302870
> +0200
> +++ gcc/testsuite/gcc.dg/pr105415.c 2022-04-28 12:25:36.770809149 +0200
> @@ -0,0 +1,26 @@
> +/* PR debug/105415 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target pthread } */
> +/* { dg-options "-O2 -ftree-parallelize-loops=2 -fcompare-debug" } */
> +
> +int m;
> +static int n;
> +
> +void
> +foo (void)
> +{
> + int s = 0;
> +
> + while (m < 1)
> + {
> + s += n;
> + ++m;
> + }
> +}
> +
> +void
> +bar (int *arr, int i)
> +{
> + while (i < 1)
> + arr[i++] = 1;
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
