* PATCH RFA: PR go/55201: Create libatomic convenience library
@ 2012-12-18 19:30 Ian Lance Taylor
2012-12-18 21:32 ` Richard Henderson
0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2012-12-18 19:30 UTC (permalink / raw)
To: Matthias Klose; +Cc: Andreas Schwab, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 757 bytes --]
On Tue, Dec 18, 2012 at 9:57 AM, Ian Lance Taylor <iant@google.com> wrote:
>
> This doesn't happen for me, and it's bizarre that libtool would turn a
> link against ../libatomic/libatomic.la into a link against -latomic.
> But in any case the fix is presumably going to be to add a convenience
> library for libatomic, as is done for libffi. I'll prepare a patch
> for that.
Like so.
OK for mainline?
If this is approved, I will submit the changes to libgo to use it.
Ian
2012-12-18 Ian Lance Taylor <iant@google.com>
PR go/55201
* Makefile.am (noinst_LTLIBRARIES): Define new make variable.
(libatomic_convenience_la_SOURCES): Likewise.
(libatomic_convenience_la_LIBADD): Likewise.
* Makefile.in: Rebuild.
* testsuite/Makefile.in: Rebuild.
[-- Attachment #2: foo.patch --]
[-- Type: application/octet-stream, Size: 656 bytes --]
Index: Makefile.am
===================================================================
--- Makefile.am (revision 194590)
+++ Makefile.am (working copy)
@@ -40,6 +40,7 @@ AM_CCASFLAGS = $(XCFLAGS)
AM_LDFLAGS = $(XLDFLAGS) $(SECTION_LDFLAGS) $(OPT_LDFLAGS)
toolexeclib_LTLIBRARIES = libatomic.la
+noinst_LTLIBRARIES = libatomic_convenience.la
if LIBAT_BUILD_VERSIONED_SHLIB
if LIBAT_BUILD_VERSIONED_SHLIB_GNU
@@ -134,3 +135,6 @@ IFUNC_OPTIONS = -mcx16
libatomic_la_LIBADD += $(addsuffix _16_1_.lo,$(SIZEOBJS))
endif
endif
+
+libatomic_convenience_la_SOURCES = $(libatomic_la_SOURCES)
+libatomic_convenience_la_LIBADD = $(libatomic_la_LIBADD)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 19:30 PATCH RFA: PR go/55201: Create libatomic convenience library Ian Lance Taylor
@ 2012-12-18 21:32 ` Richard Henderson
2012-12-18 22:09 ` Ian Lance Taylor
0 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2012-12-18 21:32 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On 12/18/2012 11:30 AM, Ian Lance Taylor wrote:
> 2012-12-18 Ian Lance Taylor <iant@google.com>
>
> PR go/55201
> * Makefile.am (noinst_LTLIBRARIES): Define new make variable.
> (libatomic_convenience_la_SOURCES): Likewise.
> (libatomic_convenience_la_LIBADD): Likewise.
> * Makefile.in: Rebuild.
> * testsuite/Makefile.in: Rebuild.
Ok.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 21:32 ` Richard Henderson
@ 2012-12-18 22:09 ` Ian Lance Taylor
2012-12-18 22:31 ` Richard Henderson
0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2012-12-18 22:09 UTC (permalink / raw)
To: Richard Henderson; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 975 bytes --]
On Tue, Dec 18, 2012 at 1:32 PM, Richard Henderson <rth@redhat.com> wrote:
> On 12/18/2012 11:30 AM, Ian Lance Taylor wrote:
>> 2012-12-18 Ian Lance Taylor <iant@google.com>
>>
>> PR go/55201
>> * Makefile.am (noinst_LTLIBRARIES): Define new make variable.
>> (libatomic_convenience_la_SOURCES): Likewise.
>> (libatomic_convenience_la_LIBADD): Likewise.
>> * Makefile.in: Rebuild.
>> * testsuite/Makefile.in: Rebuild.
>
> Ok.
Committed.
I have now committed this follow-on patch, to make libgo use the new
libatomic_convenience library. This means that the changes to
explicitly link against -latomic are no longer necessary.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.
Ian
gcc/go:
2012-12-18 Ian Lance Taylor <iant@google.com>
PR go/55201
* gospec.c: Revert last patch.
gcc/testsuite:
2012-12-18 Ian Lance Taylor <iant@google.com>
PR go/55201
* lib/go.exp: Revert last patch.
[-- Attachment #2: foo.patch --]
[-- Type: application/octet-stream, Size: 2890 bytes --]
Index: libgo/Makefile.am
===================================================================
--- libgo/Makefile.am (revision 194589)
+++ libgo/Makefile.am (working copy)
@@ -1909,7 +1909,7 @@ libgo_la_LDFLAGS = \
libgo_la_LIBADD = \
$(libgo_go_objs) ../libbacktrace/libbacktrace.la \
- ../libatomic/libatomic.la \
+ ../libatomic/libatomic_convenience.la \
$(LIBFFI) $(PTHREAD_LIBS) $(MATH_LIBS) $(NET_LIBS)
libgobegin_a_SOURCES = \
@@ -1949,7 +1949,7 @@ GOTESTFLAGS =
# Check a package.
CHECK = \
- GC="$(GOC) $(GOCFLAGS) $($(subst /,_,$@)_GOCFLAGS) -L `${PWD_COMMAND}` -L `${PWD_COMMAND}`/.libs -L `${PWD_COMMAND}`/../libatomic -L `${PWD_COMMAND}`/../libatomic/.libs"; \
+ GC="$(GOC) $(GOCFLAGS) $($(subst /,_,$@)_GOCFLAGS) -L `${PWD_COMMAND}` -L `${PWD_COMMAND}`/.libs"; \
export GC; \
GOLIBS="$(MATH_LIBS) $(NET_LIBS)"; \
export GOLIBS; \
@@ -1958,7 +1958,7 @@ CHECK = \
MAKE="$(MAKE)"; \
export MAKE; \
libgccdir=`${GOC} -print-libgcc-file-name | sed -e 's|/[^/]*$$||'`; \
- LD_LIBRARY_PATH="`${PWD_COMMAND}`/.libs:`${PWD_COMMAND}`/../libatomic/.libs:$${libgccdir}:${LD_LIBRARY_PATH}"; \
+ LD_LIBRARY_PATH="`${PWD_COMMAND}`/.libs:$${libgccdir}:${LD_LIBRARY_PATH}"; \
LD_LIBRARY_PATH=`echo $${LD_LIBRARY_PATH} | sed 's,::*,:,g;s,^:*,,;s,:*$$,,'`; \
export LD_LIBRARY_PATH; \
$(MKDIR_P) $(@D); \
Index: gcc/go/gospec.c
===================================================================
--- gcc/go/gospec.c (revision 194589)
+++ gcc/go/gospec.c (working copy)
@@ -45,9 +45,6 @@ along with GCC; see the file COPYING3.
#define THREAD_LIBRARY "pthread"
#define THREAD_LIBRARY_PROFILE THREAD_LIBRARY
-#define LIBATOMIC "atomic"
-#define LIBATOMIC_PROFILE LIBATOMIC
-
#define LIBGO "go"
#define LIBGO_PROFILE LIBGO
#define LIBGOBEGIN "gobegin"
@@ -339,11 +336,6 @@ lang_specific_driver (struct cl_decoded_
added_libraries++;
j++;
- generate_option (OPT_l, saw_profile_flag ? LIBATOMIC_PROFILE : LIBATOMIC,
- 1, CL_DRIVER, &new_decoded_options[j]);
- added_libraries++;
- j++;
-
#ifdef HAVE_LD_STATIC_DYNAMIC
if (library > 1 && !static_link)
{
Index: gcc/testsuite/lib/go.exp
===================================================================
--- gcc/testsuite/lib/go.exp (revision 194589)
+++ gcc/testsuite/lib/go.exp (working copy)
@@ -111,11 +111,6 @@ proc go_link_flags { paths } {
append flags "-L${gccpath}/libgo/.libs "
append ld_library_path ":${gccpath}/libgo/.libs"
}
- if { [file exists "${gccpath}/libatomic/.libs/libatomic.a"] \
- || [file exists "${gccpath}/libatomic/.libs/libatomic.${shlib_ext}"] } {
- append flags "-L${gccpath}/libatomic/.libs "
- append ld_library_path ":${gccpath}/libatomic/.libs"
- }
if [file exists "${gccpath}/libiberty/libiberty.a"] {
append flags "-L${gccpath}/libiberty "
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 22:09 ` Ian Lance Taylor
@ 2012-12-18 22:31 ` Richard Henderson
2012-12-18 22:52 ` Ian Lance Taylor
0 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2012-12-18 22:31 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On 12/18/2012 02:09 PM, Ian Lance Taylor wrote:
> I have now committed this follow-on patch, to make libgo use the new
> libatomic_convenience library. This means that the changes to
> explicitly link against -latomic are no longer necessary.
Hang on, what are we doing here. Are we linking libatomic statically
into libgo? That's only going to work so long as the target does not
require the use of a mutex. While that's most desktop targets, it's
certainly not all of them.
If the target requires a mutex for the atomic operation, then there
must be exactly one copy of the atomic library.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 22:31 ` Richard Henderson
@ 2012-12-18 22:52 ` Ian Lance Taylor
2012-12-18 23:09 ` Richard Henderson
2012-12-18 23:15 ` Richard Henderson
0 siblings, 2 replies; 10+ messages in thread
From: Ian Lance Taylor @ 2012-12-18 22:52 UTC (permalink / raw)
To: Richard Henderson; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On Tue, Dec 18, 2012 at 2:30 PM, Richard Henderson <rth@redhat.com> wrote:
> On 12/18/2012 02:09 PM, Ian Lance Taylor wrote:
>> I have now committed this follow-on patch, to make libgo use the new
>> libatomic_convenience library. This means that the changes to
>> explicitly link against -latomic are no longer necessary.
>
> Hang on, what are we doing here. Are we linking libatomic statically
> into libgo? That's only going to work so long as the target does not
> require the use of a mutex. While that's most desktop targets, it's
> certainly not all of them.
>
> If the target requires a mutex for the atomic operation, then there
> must be exactly one copy of the atomic library.
Argh. But why? Wouldn't that only apply to cases where the lock was
sometimes locked by one library and sometimes locked by a different
one?
Ian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 22:52 ` Ian Lance Taylor
@ 2012-12-18 23:09 ` Richard Henderson
2012-12-18 23:15 ` Richard Henderson
1 sibling, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2012-12-18 23:09 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On 12/18/2012 02:52 PM, Ian Lance Taylor wrote:
> Argh. But why? Wouldn't that only apply to cases where the lock was
> sometimes locked by one library and sometimes locked by a different
> one?
If two copies of the library aren't looking at the same lock object,
then the lock does no actual locking.
The lock object(s) themselves are not part of the public interface
of the libatomic library.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 22:52 ` Ian Lance Taylor
2012-12-18 23:09 ` Richard Henderson
@ 2012-12-18 23:15 ` Richard Henderson
2012-12-19 0:28 ` Ian Lance Taylor
1 sibling, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2012-12-18 23:15 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On 12/18/2012 02:52 PM, Ian Lance Taylor wrote:
> Argh. But why? Wouldn't that only apply to cases where the lock was
> sometimes locked by one library and sometimes locked by a different
> one?
Or did you really mean
"... only apply to cases where the memory protected by the lock
was visible to more than one library."
Yes, if libgo is attempting atomic accesses to its own data structures,
which themselves are not exported from libgo, then a copy of libatomic
ought to work.
It would probably be better for the shared libgo to depend on the
shared libatomic though. That's simply more pedantically correct.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-18 23:15 ` Richard Henderson
@ 2012-12-19 0:28 ` Ian Lance Taylor
2012-12-19 15:38 ` Matthias Klose
0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2012-12-19 0:28 UTC (permalink / raw)
To: Richard Henderson; +Cc: Matthias Klose, Andreas Schwab, gcc-patches
On Tue, Dec 18, 2012 at 3:15 PM, Richard Henderson <rth@redhat.com> wrote:
> On 12/18/2012 02:52 PM, Ian Lance Taylor wrote:
>> Argh. But why? Wouldn't that only apply to cases where the lock was
>> sometimes locked by one library and sometimes locked by a different
>> one?
>
> Or did you really mean
>
> "... only apply to cases where the memory protected by the lock
> was visible to more than one library."
>
> Yes, if libgo is attempting atomic accesses to its own data structures,
> which themselves are not exported from libgo, then a copy of libatomic
> ought to work.
>
> It would probably be better for the shared libgo to depend on the
> shared libatomic though. That's simply more pedantically correct.
But according to Matthias's comment upthread, it would require
addressing some issue in libtool in order to get multilib working
correctly.
I'm not going to try to solve this today, but if somebody else wants
to that would be great.
Ian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-19 0:28 ` Ian Lance Taylor
@ 2012-12-19 15:38 ` Matthias Klose
2012-12-19 15:47 ` Ian Lance Taylor
0 siblings, 1 reply; 10+ messages in thread
From: Matthias Klose @ 2012-12-19 15:38 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: Richard Henderson, Andreas Schwab, gcc-patches
Am 19.12.2012 01:28, schrieb Ian Lance Taylor:
> On Tue, Dec 18, 2012 at 3:15 PM, Richard Henderson <rth@redhat.com> wrote:
>> On 12/18/2012 02:52 PM, Ian Lance Taylor wrote:
>>> Argh. But why? Wouldn't that only apply to cases where the lock was
>>> sometimes locked by one library and sometimes locked by a different
>>> one?
>>
>> Or did you really mean
>>
>> "... only apply to cases where the memory protected by the lock
>> was visible to more than one library."
>>
>> Yes, if libgo is attempting atomic accesses to its own data structures,
>> which themselves are not exported from libgo, then a copy of libatomic
>> ought to work.
>>
>> It would probably be better for the shared libgo to depend on the
>> shared libatomic though. That's simply more pedantically correct.
>
> But according to Matthias's comment upthread, it would require
> addressing some issue in libtool in order to get multilib working
> correctly.
>
> I'm not going to try to solve this today, but if somebody else wants
> to that would be great.
you cannot reproduce this, if you already have libatomic.so.1 installed into
your destination/installation path. You should be able to reproduce this by
installing into an empty destination (maybe using DESTDIR). Then libgo can't
find the libatomic. calling make install-target-libatomic and then re-running
lets make install succeed.
Makefile.def doesn't define any dependencies on the installation targets,
however seldom people run make install with -j, and then the order the
target_modules are listed matters. libatomic comes behind the libgo target, otoh
libquadmath comes before libgfortran and installs without issues.
The following patch fixes this for me, maybe other target library dependencies
should be added too.
that would be for
libgfortran on libquadmath, libgcc
libsanitizer on libstdc++
libstdc++ on libgomp, libgcc
libjava on libstdc++, libgcc
libasan on libgcc
libobjc on libgcc
libitm on libgcc
Matthias
Index: Makefile.def
===================================================================
--- Makefile.def (Revision 194604)
+++ Makefile.def (Arbeitskopie)
@@ -514,6 +514,8 @@
// recursive make, we can't be that specific.
dependencies = { module=all-target-libstdc++-v3; on=configure-target-libgomp; };
+dependencies = { module=install-target-libgo; on=install-target-libatomic; };
+
// Target modules in the 'src' repository.
lang_env_dependencies = { module=libtermcap; };
lang_env_dependencies = { module=rda; };
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PATCH RFA: PR go/55201: Create libatomic convenience library
2012-12-19 15:38 ` Matthias Klose
@ 2012-12-19 15:47 ` Ian Lance Taylor
0 siblings, 0 replies; 10+ messages in thread
From: Ian Lance Taylor @ 2012-12-19 15:47 UTC (permalink / raw)
To: Matthias Klose; +Cc: Richard Henderson, Andreas Schwab, gcc-patches
On Wed, Dec 19, 2012 at 7:38 AM, Matthias Klose <doko@ubuntu.com> wrote:
>
> The following patch fixes this for me, maybe other target library dependencies
> should be added too.
>
> that would be for
> libgfortran on libquadmath, libgcc
> libsanitizer on libstdc++
> libstdc++ on libgomp, libgcc
> libjava on libstdc++, libgcc
> libasan on libgcc
> libobjc on libgcc
> libitm on libgcc
>
> Matthias
>
> Index: Makefile.def
> ===================================================================
> --- Makefile.def (Revision 194604)
> +++ Makefile.def (Arbeitskopie)
> @@ -514,6 +514,8 @@
> // recursive make, we can't be that specific.
> dependencies = { module=all-target-libstdc++-v3; on=configure-target-libgomp; };
>
> +dependencies = { module=install-target-libgo; on=install-target-libatomic; };
> +
> // Target modules in the 'src' repository.
> lang_env_dependencies = { module=libtermcap; };
> lang_env_dependencies = { module=rda; };
>
Thanks for looking into it.
This patch is OK with a ChangeLog entry.
Ian
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-19 15:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-18 19:30 PATCH RFA: PR go/55201: Create libatomic convenience library Ian Lance Taylor
2012-12-18 21:32 ` Richard Henderson
2012-12-18 22:09 ` Ian Lance Taylor
2012-12-18 22:31 ` Richard Henderson
2012-12-18 22:52 ` Ian Lance Taylor
2012-12-18 23:09 ` Richard Henderson
2012-12-18 23:15 ` Richard Henderson
2012-12-19 0:28 ` Ian Lance Taylor
2012-12-19 15:38 ` Matthias Klose
2012-12-19 15:47 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).