[Bug c++/111067] New: g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[Bug c++/111067] New: g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[fxcoudert at gcc dot gnu.org]

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

            Bug ID: 111067
           Summary: g++.dg/opt/icf{1,2,3}.C tests fail on darwin
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fxcoudert at gcc dot gnu.org
  Target Milestone: ---

As seen for example on
https://gcc.gnu.org/pipermail/gcc-testresults/2023-August/793205.html

FAIL: g++.dg/opt/icf1.C  -std=gnu++14 execution test
FAIL: g++.dg/opt/icf1.C  -std=gnu++17 execution test
FAIL: g++.dg/opt/icf1.C  -std=gnu++20 execution test
FAIL: g++.dg/opt/icf2.C  -std=gnu++14 execution test
FAIL: g++.dg/opt/icf2.C  -std=gnu++17 execution test
FAIL: g++.dg/opt/icf2.C  -std=gnu++20 execution test
FAIL: g++.dg/opt/icf3.C  -std=gnu++14 execution test
FAIL: g++.dg/opt/icf3.C  -std=gnu++17 execution test
FAIL: g++.dg/opt/icf3.C  -std=gnu++20 execution test

The tests fail on Darwin. I'm willing to help but do not know where to begin
debugging this.

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[fxcoudert at gcc dot gnu.org]

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

Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |14.0
             Target|                            |x86_64-apple-darwin20
                 CC|                            |jason at gcc dot gnu.org
               Host|                            |x86_64-apple-darwin20
              Build|                            |x86_64-apple-darwin20
   Last reconfirmed|                            |2023-08-18
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

[Bug testsuite/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |testsuite

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect this needs:
/* { dg-add-options bind_pic_locally } */

[Bug testsuite/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[fxcoudert at gcc dot gnu.org]

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

--- Comment #2 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
I tried with:

diff --git a/gcc/testsuite/g++.dg/opt/icf1.C b/gcc/testsuite/g++.dg/opt/icf1.C
index fbb275e635a..d4e4bbf91b9 100644
--- a/gcc/testsuite/g++.dg/opt/icf1.C
+++ b/gcc/testsuite/g++.dg/opt/icf1.C
@@ -1,6 +1,7 @@
 // Test that -fipa-icf combines i and j.
 // { dg-do run { target c++11 } }
 // { dg-options -fipa-icf }
+// { dg-add-options bind_pic_locally }

 [[no_unique_address]] extern const int i[] = { 1,2,3 };
 [[no_unique_address]] extern const int j[] = { 1,2,3 };

and similarly on the other files, but it does not make the tests pass.

[Bug testsuite/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

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

--- Comment #3 from Iain Sandoe <iains at gcc dot gnu.org> ---
I took a brief look at this over the weekend, but I need to check what is
allowed/expected by the ABI - we normally reject cases where two
externally-visible symbols have the same address, since that breaks the ld64
'atom model'; how to reconcile that constraint with 'no unique address' is
interesting.  

We do support merging literals, and we can support aliased symbols by
instruction to the linker that they are - so there might be viable way(s) to
implement.

[Bug testsuite/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[iains at gcc dot gnu.org]

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

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
clang on Darwin rejects the code thus:

$ ./bin/clang++ /src-local/gcc-master/gcc/testsuite/g++.dg/opt/icf1.C -o i1
-std=c++14
/src-local/gcc-master/gcc/testsuite/g++.dg/opt/icf1.C:5:3: error:
'no_unique_address' attribute only applies to non-bit-field non-static data
members
    5 | [[no_unique_address]] extern const int i[] = { 1,2,3 };

this seems to agree with the current WD:
https://eel.is/c++draft/dcl.attr.nouniqueaddr

"9.12.11 No unique address attribute [dcl.attr.nouniqueaddr]

1 #
The attribute-token no_unique_address specifies that a non-static data member
is a potentially-overlapping subobject ([intro.object]). No
attribute-argument-clause shall be present. The attribute may appertain to a
non-static data member other than a bit-field.
"

since the objects in the test case are not non-static, non-bitfield data
members, any application of the attribute would be a GCC extension, I guess?

What is the intent?

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|x86_64-apple-darwin20       |*-darwin*
          Component|testsuite                   |c++
   Last reconfirmed|2023-08-18 00:00:00         |2024-3-7
               Host|x86_64-apple-darwin20       |*-darwin*
              Build|x86_64-apple-darwin20       |*-darwin*

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
I am shifting this to component C++, because the answer to comment #4 is really
what's important - if this is supposed to work for file scope entities, then it
needs to be a GCC extension (ISTM, anyway).

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, I think it's an extension, added by r14-1500-g4d935f52b0d5c0

The question then is whether the attribute is supposed to be a non-binding
request or not.

If it's a non-binding request then the test should be adjusted/unsupported for
this target. If the latter, then that use of the attribute should be ill-formed
for this target and give an error.

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[iains at gcc dot gnu.org]

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

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
however:

 1) it is not in the gnu:: namespace the tests just have it as [[ ]].
 2) I do not think that the standard has anything to say about how entities at
file scope are ordered in memory (many/most impls. probably use the same order
as the file - but I do not think they are obliged to).
3) it is an ABI break for Darwin (where two global entities are not allowed to
have the same address - although it _is_ possible to tell the linker that one
is an alias for another - that support is not (yet) available in the FE).

So I am actually asking 
 - if the extension actually has any useful meaning?
 - if it is an extension it probably should be only available in gnu:: ?

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #7)
> So I am actually asking 
>  - if the extension actually has any useful meaning?

For non-darwin, yes, it requests the storage of two initializer lists to be
merged (see the commit msg for r14-1500-g4d935f52b0d5c0).

>  - if it is an extension it probably should be only available in gnu:: ?

It's a standard attribute. The extension is allowing it to be placed on
variables that aren't non-static data members. If you spell it
gnu::no_unique_address then it's a completely different attribute (and in that
case I'd suggest an alternative name, because we don't want to recreate the
msvc::no_unique_address debacle, or even give the impression of doing so!)

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #7)
> So I am actually asking 
>  - if the extension actually has any useful meaning?

YES it does. Especially when it comes to the ability to reduce space used.
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2752r2.html is the
link to the paper that is referenced as the notion for the extension.

I think this is a minor issue from the point of view of the testcase can just
be xfailed on darwin really.

Though I suspect the backend will/should be fixed to mark the alias as being
the same symbols really.

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[jason at gcc dot gnu.org]

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

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #8)
> (In reply to Iain Sandoe from comment #7)
> > So I am actually asking if the extension actually has any useful meaning?
> 
> For non-darwin, yes, it requests the storage of two initializer lists to be
> merged (see the commit msg for r14-1500-g4d935f52b0d5c0).

Though that doesn't involve the attribute, and promoting init-lists to static
should work fine on darwin.

(In reply to Jonathan Wakely from comment #6)
> The question then is whether the attribute is supposed to be a non-binding
> request or not.
> 
> If it's a non-binding request then the test should be adjusted/unsupported
> for this target.

It is a non-binding request. And yes, if this optimization is problematic on
darwin, we should adjust the test.

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[iains at gcc dot gnu.org]

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

--- Comment #11 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #10)
> (In reply to Jonathan Wakely from comment #8)
> > (In reply to Iain Sandoe from comment #7)
> > > So I am actually asking if the extension actually has any useful meaning?
> > 
> > For non-darwin, yes, it requests the storage of two initializer lists to be
> > merged (see the commit msg for r14-1500-g4d935f52b0d5c0).
> 
> Though that doesn't involve the attribute, and promoting init-lists to
> static should work fine on darwin.

I think that is working we end up with two constant text arrays (no copy via
automatic storage as mentioned in the paper)

> (In reply to Jonathan Wakely from comment #6)
> > The question then is whether the attribute is supposed to be a non-binding
> > request or not.
> > 
> > If it's a non-binding request then the test should be adjusted/unsupported
> > for this target.
> 
> It is a non-binding request. And yes, if this optimization is problematic on
> darwin, we should adjust the test.

Actually, the optimisation is failing on Darwin - we produce two distinct
arrays.

(although, if it succeeds then technically that's breaking the ABI since we
then have two external symbols with the same addresss).

SO I suppose the question is do we want to figure out why the opt is failing
(knowing that if it succeeds that is a secondary issue) - or just
dg-xfail-run-if for Darwin?

This is what we generate now:
        .const
        .align 3
_i:
        .long   1
        .long   2
        .long   3
        .globl _j
        .align 3
_j:
        .long   1
        .long   2
        .long   3

and we do access i and j directly (i.e. not via the GOT).

[Bug c++/111067] g++.dg/opt/icf{1,2,3}.C tests fail on darwin

[jason at gcc dot gnu.org]

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

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #11)
> SO I suppose the question is do we want to figure out why the opt is failing
> (knowing that if it succeeds that is a secondary issue) - or just
> dg-xfail-run-if for Darwin?

I think we can just disable icf1 on darwin, and not try to merge i and j.

It would be nice for icf[23] to work, since those don't involve
externally-visible symbols (or even names at all).  It would be good to
understand what is preventing the optimization in that case.