[Bug tree-optimization/113054] New: [14 regressions] ODR warnings when building new SCCP pass

[Bug tree-optimization/113054] New: [14 regressions] ODR warnings when building new SCCP pass

[sjames at gcc dot gnu.org]

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

            Bug ID: 113054
           Summary: [14 regressions] ODR warnings when building new SCCP
                    pass
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sjames at gcc dot gnu.org
                CC: fkastl at suse dot cz
  Target Milestone: ---

The new SCCP pass introduces some new -Wodr warnings:
```
[...]
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/gimple-ssa-sccopy.cc:121:
note: a field with different name is defined in another translation unit
  121 |   bool active; /* scc_discovery::compute_sccs () only considers a
subgraph of
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/graphds.h:35:8:
warning: type ‘struct vertex’ violates the C++ One Definition Rule [-Wodr]
   35 | struct vertex
      |        ^
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/gimple-ssa-sccopy.cc:119:
note: a different type is defined in another translation unit
  119 | struct vertex
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/graphds.h:37:22:
note: the first difference of corresponding definitions is field ‘pred’
   37 |   struct graph_edge *pred, *succ;
      |                      ^
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/gimple-ssa-sccopy.cc:121:
note: a field with different name is defined in another translation unit
  121 |   bool active; /* scc_discovery::compute_sccs () only considers a
subgraph of
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/graphds.h:35:
warning: type ‘struct vertex’ violates the C++ One Definition Rule [-Wodr]
   35 | struct vertex
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/gimple-ssa-sccopy.cc:119:
note: a different type is defined in another translation unit
  119 | struct vertex
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/graphds.h:37:
note: the first difference of corresponding definitions is field ‘pred’
   37 |   struct graph_edge *pred, *succ;
/var/tmp/portage/sys-devel/gcc-14.0.0_pre20231217/work/gcc-14-20231217/gcc/gimple-ssa-sccopy.cc:121:
note: a field with different name is defined in another translation unit
  121 |   bool active; /* scc_discovery::compute_sccs () only considers a
subgraph of
[...]
```

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
My suggestion is to wrap most of the code in gimple-ssa-sccopy.cc inside an
anonymous namespace.

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I also don't like how dead_stmts is a static variable either but that
would be for another change.

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Target Milestone|---                         |14.0
   Last reconfirmed|                            |2023-12-18
     Ever confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a patch.

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 56896
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56896&action=edit
Patch which I am testing

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:4554a151d0ec62332c332175ec1017f853006b60

commit r14-6663-g4554a151d0ec62332c332175ec1017f853006b60
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Mon Dec 18 08:18:13 2023 -0800

    SCCP: Fix ODR issues when compiling with LTO [PR 113054}

    The problem here is that in C++ structs and classes have a
    linkage too so the type vertex is not considered local to
    the TU but will conflict with the globally defined one
    in graphds.h.  The simple way to fix this is to wrap the ones
    defined locally in gimple-ssa-sccopy.cc inside an anonymous namespace
    and they are now considered locally to that TU.

    Committed as obvious after a bootstrap/test on x86_64.

    gcc/ChangeLog:

            PR tree-optimization/113054
            * gimple-ssa-sccopy.cc: Wrap the local types
            with an anonymous namespace.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[fkastl at suse dot cz]

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

--- Comment #7 from Filip Kastl <fkastl at suse dot cz> ---
Thanks for the fix! Will be careful not to trigger ODR with my future patches.

(In reply to Andrew Pinski from comment #2)
> Note I also don't like how dead_stmts is a static variable either but that
> would be for another change.

How should I get rid of the static variable? Maybe wrap the copy propagation
algorithm in a class as I wrapped the SCC finding algorithm?

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Filip Kastl from comment #7)
> (In reply to Andrew Pinski from comment #2)
> > Note I also don't like how dead_stmts is a static variable either but that
> > would be for another change.
> 
> How should I get rid of the static variable? Maybe wrap the copy propagation
> algorithm in a class as I wrapped the SCC finding algorithm?

Yes that would most likely be the way I would do it. and then
init_sccopy/finalize_sccopy becomes constructors/deconstructors ...

[Bug tree-optimization/113054] [14 regressions] ODR warnings when building new SCCP pass

[fkastl at suse dot cz]

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

--- Comment #9 from Filip Kastl <fkastl at suse dot cz> ---
Alright. I suppose this change wouldn't be appropriate in stage 3 nor stage 4,
so I'll wait for the next stage 1 and modify sccopy to use a class.