[Bug c++/94223] New: [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[Bug c++/94223] New: [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[zhroma at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

            Bug ID: 94223
           Summary: [10 Regression] -fcompare-debug -O0 failure on
                    cpp1z/init-statement6.C
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhroma at gcc dot gnu.org
  Target Milestone: ---

This commands:
gcc -c -O0 -std=c++20 -fcompare-debug
gcc/testsuite/g++.dg/cpp1z/init-statement6.C
gcc -c -O0 -std=c++2a -fcompare-debug
gcc/testsuite/g++.dg/cpp1z/init-statement6.C

fail on master (r10-7271) on ppc64le, work on 9.3 (only have -std=c++2a) on
x86_64.

Running with -O1, -O2 or -std=c++17 goes smoothly.

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
gimplify.c does:
4941                if (!DECL_NAME (object))
4942                  DECL_NAME (object) = create_tmp_var_name ("C");
4943                walk_tree (&DECL_INITIAL (object), force_labels_r, NULL,
NULL);
4944    
4945                /* ??? C++ doesn't automatically append a .<number> to the
4946                   assembler name, and even when it does, it looks at FE
private
4947                   data structures to figure out what that number should
be,
4948                   which are not set for this variable.  I suppose this is
4949                   important for local statics for inline functions, which
aren't
4950                   "local" in the object file sense.  So in order to get a
unique
4951                   TU-local symbol, we must invoke the lhd version now.  */
4952                lhd_set_decl_assembler_name (object);

and lhd_set_decl_assembler_name does:
169           const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
170           char *label;
171     
172           ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
173           id = get_identifier (label);

The problem is that DECL_UID isn't stable across -fcompare-debug, we only
guarantee that the comparisons of the DECL_UIDs are the same between
corresponding decls, but -g can have larger gaps.

grep ASM_FORMAT_PRIVATE_NAME.*DECL_UID * */* config/*/* 2>/dev/null
langhooks.c:      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
d/decl.cc:  ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));
lto/lto-lang.c:      ASM_FORMAT_PRIVATE_NAME (label, name, DECL_UID (decl));

Not sure what to do, shall we have some GTY counter used for these instead of
DECL_UID which we'd bump when we've done this?

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.0
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
lhd_set_decl_assembler_name seems to only do this for local decls though
so it shouldn't matter for actual generated code but is just a
compare-debug artifact?  If it matters for code-generation then yes,
a local counter should do (does it really need to be GTY?)

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> lhd_set_decl_assembler_name seems to only do this for local decls though
> so it shouldn't matter for actual generated code but is just a
> compare-debug artifact?  If it matters for code-generation then yes,
> a local counter should do (does it really need to be GTY?)

Yes, it does that only for local decls, so partly it is a -fcompare-debug
artifact (shows up e.g. in the (symbol_ref:DI "C.4.1983454")) and partly it is
observable in the symbol table.  The -fcompare-debug checking comparison
doesn't print DECL_UIDs, but in these cases where it is DECL_ASSEMBLER_NAME or
something that makes it into RTL it just doesn't know that it a part of the
identifier was formed from a DECL_UID.
As for GTY, maybe we don't need it.

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note the code is the same on the GCC 9 branch so the issue must be latent maybe
with different testcases.

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48106
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48106&action=edit
gcc10-pr94223.patch

Untested fix.

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:158cccea0d097d9f181bf4e35fdeb97865c960f7

commit r10-7367-g158cccea0d097d9f181bf4e35fdeb97865c960f7
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 25 09:18:33 2020 +0100

    middle-end: Avoid using DECL_UID in ASM_FORMAT_PRIVATE_NAME [PR94223]

    As mentioned in the PR, we don't guarantee DECL_UID to be the same between
    corresponding decls in -g and -g0 builds, -g can create more decls and all
    that is guaranteed is that the DECL_UIDs of the corresponding decls compare
    the same.
    The following testcase gets a -fcompare-debug failure because these
    functions use DECL_UID as the number in ASM_FORMAT_PRIVATE_NAME.

    The patch fixes it by using just a sequential number there instead.
    I don't think this can be called during PCH writing, this only happens for
    non-public decls and the C/C++ FEs shouldn't mangling those at that point
    (furthermore C++ FE uses a different set_decl_assembler_name hook and this
    one is something only the gimplifier calls on C.NNNN temporaries.

    2020-03-25  Jakub Jelinek  <jakub@redhat.com>

            PR c++/94223
            * langhooks.c (lhd_set_decl_assembler_name): Use a static ulong
            counter instead of DECL_UID.

            * lto-lang.c (lto_set_decl_assembler_name): Use a static ulong
            counter instead of DECL_UID.

            * g++.dg/opt/pr94223.C: New test.

[Bug c++/94223] [10 Regression] -fcompare-debug -O0 failure on cpp1z/init-statement6.C

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94223

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.