[Bug debug/104407] New: [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[Bug debug/104407] New: [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[asolokha at gmx dot com]

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

            Bug ID: 104407
           Summary: [10/11/12 Regression] '-fcompare-debug' failure
                    (length) w/ -std=c++17 -O1
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: compare-debug-failure
          Severity: normal
          Priority: P3
         Component: debug
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: x86_64-unknown-linux-gnu

gcc 12.0.1 20220130 snapshot (g:baf98320ac6cd56da0c0b460fb94e3b87a79220d) fails
-fcompare-debug check when compiling the following testcase, extracted from
gcc/testsuite/g++.dg/cpp1z/decomp13.C, w/ -O1:

struct A { int i; long j; int k : 2; char l; } a;

auto [ aa, bb, cc, dd ] = a;

namespace N
{
  auto & [ m, n, o, ppp ] = a;
}

% x86_64-unknown-linux-gnu-g++-12.0.1 -O1 -fcompare-debug -c t3pwb1uu.C
x86_64-unknown-linux-gnu-g++-12.0.1: error: t3pwb1uu.C: '-fcompare-debug'
failure (length)

--- t3pwb1uu.C.gkd      2022-02-06 14:31:15.936434434 +0700
+++ t3pwb1uu.gk.C.gkd   2022-02-06 14:31:15.996430437 +0700
@@ -11,7 +11,7 @@
 (note # 0 0 NOTE_INSN_FUNCTION_BEG)
 (note # 0 0 NOTE_INSN_DELETED)
 (insn # 0 0 2 (set (reg:TI 20 xmm0 [89])
-        (mem/c:TI (symbol_ref:DI ("a") [flags 0x2]  <var_decl # a>) [ a+0 S16
A128])) "t3pwb1uu.C":3:6# {*movti_internal}
+        (mem/c:TI (symbol_ref:DI ("a") [flags 0x2]  <var_decl # a>) [ a+0 S16
A64])) "t3pwb1uu.C":3:6# {*movti_internal}
      (nil))
 (insn # 0 0 2 (set (mem/c:TI (symbol_ref:DI ("_ZDC2aa2bb2cc2ddE") [flags 0x2] 
<var_decl # D.xxxx>) [ D.xxxx+0 S16 A128])
         (reg:TI 20 xmm0 [89])) "t3pwb1uu.C":3:6# {*movti_internal}
@@ -19,9 +19,9 @@
         (nil)))
 (insn # 0 0 2 (set (reg:DI 0 ax [orig:87 a+16 ] [87])
         (mem/c:DI (const:DI (plus:DI (symbol_ref:DI ("a") [flags 0x2] 
<var_decl # a>)
-                    (const_int 16 [0x10]))) [ a+16 S8 A128]))
"t3pwb1uu.C":3:6# {*movdi_internal}
+                    (const_int 16 [0x10]))) [ a+16 S8 A64])) "t3pwb1uu.C":3:6#
{*movdi_internal}
      (expr_list:REG_EQUIV (mem/c:DI (const:DI (plus:DI (symbol_ref:DI ("a")
[flags 0x2]  <var_decl # a>)
-                    (const_int 16 [0x10]))) [ a+16 S8 A128])
+                    (const_int 16 [0x10]))) [ a+16 S8 A64])
         (nil)))
 (insn # 0 0 2 (set (mem/c:DI (const:DI (plus:DI (symbol_ref:DI
("_ZDC2aa2bb2cc2ddE") [flags 0x2]  <var_decl # D.xxxx>)
                     (const_int 16 [0x10]))) [ D.xxxx+16 S8 A128])

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.4
           Keywords|                            |needs-bisection
   Last reconfirmed|                            |2022-02-06
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
      Known to fail|                            |10.3.0, 11.1.0
      Known to work|                            |10.2.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, the alignment with -g is 64 for the load but 128 without -g.

-g:
(insn 9 8 10 (set (reg/f:DI 83)
        (symbol_ref:DI ("a") [flags 0x2]  <var_decl 0x7f642e3a7cf0 a>))
"/app/example.cpp":4:6 -1
     (nil))

(insn 10 9 11 (set (reg:TI 84)
        (mem/c:TI (reg/f:DI 83) [1 a+0 S16 A64])) "/app/example.cpp":4:6 -1
     (nil))

vs:
(insn 6 5 7 (set (reg/f:DI 83)
        (symbol_ref:DI ("a") [flags 0x2]  <var_decl 0x7f69102a2cf0 a>))
"/app/example.cpp":4:6 -1
     (nil))

(insn 7 6 8 (set (reg:TI 84)
        (mem/c:TI (reg/f:DI 83) [1 a+0 S16 A128])) "/app/example.cpp":4:6 -1
     (nil))


The alignment in the assembly is 16 (128) for both:
        .align 16
        .type   a, @object
        .size   a, 24
a:
        .zero   24

It not obvious what is causing the issue though.

Here is a C++98 testcase that does not depend on C++17 features.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|debug                       |target

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Also this works correctly on aarch64, so I am thinking this might be a target
issue either in the backend or during expand.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r11-2834-g7fe2cec41bb2ccb499b6b6c513e00da1a270370f

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
DECL_RTL is set in the -g case prematurely during:
#0  set_decl_rtl (t=<var_decl 0x7fffea24dcf0 a>, x=0x7fffea3b9bb8) at
../../gcc/emit-rtl.cc:1440
#1  0x0000000001bdc0a5 in make_decl_rtl (decl=<var_decl 0x7fffea24dcf0 a>) at
../../gcc/varasm.cc:1633
#2  0x000000000120cb23 in expand_expr_real_1 (exp=<var_decl 0x7fffea24dcf0 a>,
target=0x0, tmode=E_DImode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0,
inner_reference_p=false)
    at ../../gcc/expr.cc:10551
#3  0x0000000001204aaf in expand_expr_real (exp=<var_decl 0x7fffea24dcf0 a>,
target=0x0, tmode=E_DImode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0,
inner_reference_p=false)
    at ../../gcc/expr.cc:8736
#4  0x00000000011e20e8 in expand_expr (exp=<var_decl 0x7fffea24dcf0 a>,
target=0x0, mode=E_DImode, modifier=EXPAND_INITIALIZER) at ../../gcc/expr.h:301
#5  0x0000000001203a46 in expand_expr_addr_expr_1 (exp=<var_decl 0x7fffea24dcf0
a>, target=0x0, tmode=DImode, modifier=EXPAND_INITIALIZER, as=0 '\000') at
../../gcc/expr.cc:8427
#6  0x0000000001204402 in expand_expr_addr_expr (exp=<addr_expr
0x7fffea3a0e80>, target=0x0, tmode=E_DImode, modifier=EXPAND_INITIALIZER) at
../../gcc/expr.cc:8548
#7  0x0000000001212ed7 in expand_expr_real_1 (exp=<addr_expr 0x7fffea3a0e80>,
target=0x0, tmode=E_VOIDmode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0,
inner_reference_p=false)
    at ../../gcc/expr.cc:11767
#8  0x0000000001204aaf in expand_expr_real (exp=<addr_expr 0x7fffea3a0e80>,
target=0x0, tmode=E_VOIDmode, modifier=EXPAND_INITIALIZER, alt_rtl=0x0,
inner_reference_p=false)
    at ../../gcc/expr.cc:8736
#9  0x000000000112950c in expand_expr (exp=<addr_expr 0x7fffea3a0e80>,
target=0x0, mode=E_VOIDmode, modifier=EXPAND_INITIALIZER) at
../../gcc/expr.h:301
#10 0x00000000011595e0 in rtl_for_decl_init (init=<addr_expr 0x7fffea3a0e80>,
type=<reference_type 0x7fffea3a9e70>) at ../../gcc/dwarf2out.cc:20488
#11 0x000000000115ab99 in tree_add_const_value_attribute (die=<dw_die_ref
0x7fffea26a3c0 DW_TAG_variable <parent=0x7fffea26a370 DW_TAG_namespace>>,
t=<nop_expr 0x7fffea3a0ea0>)
    at ../../gcc/dwarf2out.cc:20915

Unfortunately it is deep in the expansion code, so we can't easily make it if
(!early_dwarf) don't SET_DECL_RTL, because we don't really know that this is
from early dwarf.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps we could introduce EXPAND_EARLY_DEBUG and use that instead of
EXPAND_NORMAL, but it would be quite large change.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I mean EXPAND_INITIALIZER.  Anyway, we'd have to adjust most of those ~ 35
spots referencing EXPAND_INITIALIZER in expr.cc.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, we are trying very hard to _not_ do any RTL expansion during early_dwarf
exactly because of this.  I suppose there might be a path that slipped through
...

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
One way would be either not to call rtl = expand_expr (init, NULL_RTX,
VOIDmode, EXPAND_INITIALIZER); during early_dwarf or do some extra checks
whether it is safe.  I see for the early_dwarf case it does
native_encode_initializer through which obviously nothing that needs relocation
like addresses of vars will make it through, so e.g. punting for early_dwarf
before expand_expr if it refers to any decls would do the job.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Like either for early_dwarf in reference_to_unused return *tp; for any DECL_P
or STRING_CST found, or for early_dwarf use a different callback that does
that.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenth at gcc dot gnu.org]

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
So at least

  /* Generate the RTL even if early_dwarf to force mangling of all refered to
     symbols.  */
  rtl = rtl_for_decl_init (init, type);
  if (rtl && !early_dwarf)

doesn't apply anymore since we mangle all globals in free-lang-data.  But of
course we do not actually run free-lang-data without -flto ...
assign_assembler_name_if_needed is the part that's needed.  I suppose we
could, in dwarf2out, do a walk_tree on the initializer and call that
function on each decl.

[Bug target/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> One way would be either not to call rtl = expand_expr (init, NULL_RTX,
> VOIDmode, EXPAND_INITIALIZER); during early_dwarf or do some extra checks
> whether it is safe.  I see for the early_dwarf case it does
> native_encode_initializer through which obviously nothing that needs
> relocation like addresses of vars will make it through, so e.g. punting for
> early_dwarf before expand_expr if it refers to any decls would do the job.

Yes, early dwarf may not encode references to other decls - those may go
away when optimizing.  It may only refer to those via dwarf, but not via
locations and symbol-refs.

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52362
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52362&action=edit
gcc12-pr104407.patch

This works for me.

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #12)
> Created attachment 52362 [details]
> gcc12-pr104407.patch
> 
> This works for me.

Yes.  Note we're throwing away the result anyway but the comment at the caller
hints at the expansion being necessary because that triggers mangling.  Now,
with your change we won't ever do any mangling and thus we can instead avoid
calling the function at all?

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, you're right, I thought the rtl is used in the native_encode_initializer
case but it is not.
So, if the point of r11-2834 was to mangle the referenced symbols, then we
shouldn't call rtl_for_decl_init but instead do STRIP_NOPS + if
(initializer_constant_valid_p) walk_tree with a callback that will
DECL_ASSEMBLER_NAME on the decls.

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenther at suse dot de]

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

--- Comment #15 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 7 Feb 2022, jakub at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104407
> 
> --- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Ah, you're right, I thought the rtl is used in the native_encode_initializer
> case but it is not.
> So, if the point of r11-2834 was to mangle the referenced symbols, then we
> shouldn't call rtl_for_decl_init but instead do STRIP_NOPS + if
> (initializer_constant_valid_p) walk_tree with a callback that will
> DECL_ASSEMBLER_NAME on the decls.

Yeah, I guess so, using assign_assembler_name_if_needed ()

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #52362|0                           |1
        is obsolete|                            |

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52364
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52364&action=edit
gcc12-pr104407.patch

Updated patch.  Hope it doesn't do more harm for -fcompare-debug, I'm not
really sure if e.g. mangling something doesn't force some instantiations that
wouldn't otherwise happen.  And this patch can mangle even things that weren't
mangled before with the PR96690 change, though I guess reference_to_unused
calls during early_dwarf are just random lottery.

[Bug debug/104407] [10/11/12 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #17 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:be9cd0ca8a5f13cfee6a39b217d439a25c53553a

commit r12-7129-gbe9cd0ca8a5f13cfee6a39b217d439a25c53553a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Feb 9 15:17:52 2022 +0100

    dwarf2out: Don't call expand_expr during early_dwarf [PR104407]

    As mentioned in the PR, since PR96690 r11-2834 we call rtl_for_decl_init
    which can call expand_expr already during early_dwarf.  The comment and PR
    explains it that the intent is to ensure the referenced vars and functions
    are properly mangled because free_lang_data doesn't cover everything, like
    template parameters etc.  It doesn't work well though, because expand_expr
    can set DECL_RTLs e.g. on referenced vars and keep them there, and they can
    be created e.g. with different MEM_ALIGN compared to what they would be
    created with if they were emitted later.
    So, the following patch stops calling rtl_for_decl_init and instead
    for cases for which rtl_for_decl_init does anything at all walks the
    initializer and ensures referenced vars or functions are mangled.

    2022-02-09  Jakub Jelinek  <jakub@redhat.com>

            PR debug/104407
            * dwarf2out.cc (mangle_referenced_decls): New function.
            (tree_add_const_value_attribute): Don't call rtl_for_decl_init if
            early_dwarf.  Instead walk the initializer and try to mangle vars
or
            functions referenced from it.

            * g++.dg/debug/dwarf2/pr104407.C: New test.

[Bug debug/104407] [10/11 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11/12 Regression]       |[10/11 Regression]
                   |'-fcompare-debug' failure   |'-fcompare-debug' failure
                   |(length) w/ -std=c++17 -O1  |(length) w/ -std=c++17 -O1

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug debug/104407] [10/11 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug debug/104407] [11 Regression] '-fcompare-debug' failure (length) w/ -std=c++17 -O1

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.