Re: [PATCH] gold: Use autotools pthread macro

2018-02-19 Thread Cary Coutant
> config-patches has nothing to do with the GCC config/ directory. It is
> the place to send patches for config.{guess,sub}. Taking if off the
> cc: line.

Sorry, Ben. I've started a new thread on gcc-patches for the config/
part of this patch.

https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01138.html

-cary


Re: [PATCH] gold: Use autotools pthread macro

2018-02-19 Thread Ben Elliston
On Mon, Feb 19, 2018 at 12:39:47PM -0800, Cary Coutant wrote:

> I've added config-patches to the conversation, so I'm hoping that
> was sufficient to get that going. Once added in the GCC tree, I
> think one can either wait for an automatic sync, or go ahead and
> submit a patch to do the sync (but I'm not completely versed on how
> the shared directories really operate).

config-patches has nothing to do with the GCC config/ directory. It is
the place to send patches for config.{guess,sub}. Taking if off the
cc: line.

Ben


signature.asc
Description: PGP signature


Re: [PATCH] gold: Use autotools pthread macro

2018-02-19 Thread Cary Coutant
>> First, do you or your company have a copyright assignment on file with FSF?
>
> I do not. What is the process for that? I don't need a company
> assignment, an individual contributor for me will be fine.
>
> If I sign that for this project, would it also cover GCC, or do I need
> one for each?

It will cover all FSF contributions.

If config/ax_pthread.m4 is accepted by the config maintainers, I doubt
you'd need an assignment for that, since it's already licensed and
it's not actually your contribution.

That leaves a fairly small change to gold sources (not counting
auto-regenerated files) -- small enough that I don't think you need to
complete an assignment.

If  you do want to file a copyright assignment for future changes,
I'll forward you a copy of the form and instructions separately.

>> I could be wrong, but I believe adding to config/ will require
>> approval from a GCC config/ maintainer. The normal process, as I
>> understand it, would be to add the file to the GCC tree, then sync it
>> into the binutils tree. I'm not in a position to approve that, nor can
>> I judge on the acceptability of the copyright notice in that file.
>
> Ok. I didn't realize the config/ directory came from GCC. I'll look
> into getting it submitted there How does that get "synced" to
> binutils? Would I make a patch to add it here after it is added there?

I've added config-patches to the conversation, so I'm hoping that was
sufficient to get that going. Once added in the GCC tree, I think one
can either wait for an automatic sync, or go ahead and submit a patch
to do the sync (but I'm not completely versed on how the shared
directories really operate).

> FWIW, it is the same license as the ax_check_define macro (also from
> the autotools macro archive) that is already in config/

That suggests to me that there shouldn't be a problem adding this new file.

>> I'd like to retain the ability to use --disable-threads as a configure 
>> option.
>
> I will add that back in.

Thanks.

>> If this is just to get MinGW on board, is there a lighter-weight way
>> of doing that? Could we just add a configure option that switches
>> between -lpthread and -pthread (or whatever option is needed)? I like
>> the idea of fully automating it, but that's a pretty big m4 file!
>
> It is pretty large I pulled it straight from the autoconf macro
> archive. IMHO, its much better quality than anything I would be able
> to come up with otherwise, and pretty brain-dead easy for the end
> user. It should "just work" without any special switches (although the
> user *can* configure it with some env-vars I think).

Sounds good to me if others agree.

>> (BTW, In the future, please omit diffs from generated files from your patch.)
>
> Will do. I couldn't find a lot of examples of config changes in
> binutils, but I thought the one that I did updated the generated files
> also. I must have been mistaken.

It's a convention that makes reviewing and applying patches easier for
us maintainers, but it's easy to forget to strip out those diffs.

-cary


Re: [PATCH] gold: Use autotools pthread macro

2018-02-18 Thread Joshua Watt
On Sat, Feb 17, 2018 at 4:42 PM, Cary Coutant  wrote:
>> The autotools library macro (AX_PTHREAD) is now used to detect if
>> pthreads is present and multi-threaded linking in gold is automatically
>> enabled if it is found. This enables multi-threaded gold on platforms
>> where pthreads is enabled via other methods than just -lpthread (e.g.
>> MinGW)
>>
>> Signed-off-by: Joshua Watt 
>> ---
>>  config/ax_pthread.m4   | 485 
>>  gold/Makefile.am   |  11 +-
>>  gold/Makefile.in   |  22 +-
>>  gold/aclocal.m4|   1 +
>>  gold/configure | 767 
>> +++--
>>  gold/configure.ac  |  23 +-
>>  gold/testsuite/Makefile.in |   8 +-
>>  7 files changed, 1254 insertions(+), 63 deletions(-)
>
> Thanks for the patch! I have a few preliminary questions and comments...
>
> First, do you or your company have a copyright assignment on file with FSF?

I do not. What is the process for that? I don't need a company
assignment, an individual contributor for me will be fine.

If I sign that for this project, would it also cover GCC, or do I need
one for each?

>
> I could be wrong, but I believe adding to config/ will require
> approval from a GCC config/ maintainer. The normal process, as I
> understand it, would be to add the file to the GCC tree, then sync it
> into the binutils tree. I'm not in a position to approve that, nor can
> I judge on the acceptability of the copyright notice in that file.

Ok. I didn't realize the config/ directory came from GCC. I'll look
into getting it submitted there How does that get "synced" to
binutils? Would I make a patch to add it here after it is added there?

FWIW, it is the same license as the ax_check_define macro (also from
the autotools macro archive) that is already in config/

>
> I'd like to retain the ability to use --disable-threads as a configure option.
>

I will add that back in.

> If this is just to get MinGW on board, is there a lighter-weight way
> of doing that? Could we just add a configure option that switches
> between -lpthread and -pthread (or whatever option is needed)? I like
> the idea of fully automating it, but that's a pretty big m4 file!

It is pretty large I pulled it straight from the autoconf macro
archive. IMHO, its much better quality than anything I would be able
to come up with otherwise, and pretty brain-dead easy for the end
user. It should "just work" without any special switches (although the
user *can* configure it with some env-vars I think).

>
> Anyway, I'd like to hear what the GCC folks think of adding
> ax_pthread.m4 to the config/ directory.
>
> (BTW, In the future, please omit diffs from generated files from your patch.)

Will do. I couldn't find a lot of examples of config changes in
binutils, but I thought the one that I did updated the generated files
also. I must have been mistaken.

>
> -cary

Thanks,
Joshua Watt


Re: [PATCH] gold: Use autotools pthread macro

2018-02-17 Thread Cary Coutant
> The autotools library macro (AX_PTHREAD) is now used to detect if
> pthreads is present and multi-threaded linking in gold is automatically
> enabled if it is found. This enables multi-threaded gold on platforms
> where pthreads is enabled via other methods than just -lpthread (e.g.
> MinGW)
>
> Signed-off-by: Joshua Watt 
> ---
>  config/ax_pthread.m4   | 485 
>  gold/Makefile.am   |  11 +-
>  gold/Makefile.in   |  22 +-
>  gold/aclocal.m4|   1 +
>  gold/configure | 767 
> +++--
>  gold/configure.ac  |  23 +-
>  gold/testsuite/Makefile.in |   8 +-
>  7 files changed, 1254 insertions(+), 63 deletions(-)

Thanks for the patch! I have a few preliminary questions and comments...

First, do you or your company have a copyright assignment on file with FSF?

I could be wrong, but I believe adding to config/ will require
approval from a GCC config/ maintainer. The normal process, as I
understand it, would be to add the file to the GCC tree, then sync it
into the binutils tree. I'm not in a position to approve that, nor can
I judge on the acceptability of the copyright notice in that file.

I'd like to retain the ability to use --disable-threads as a configure option.

If this is just to get MinGW on board, is there a lighter-weight way
of doing that? Could we just add a configure option that switches
between -lpthread and -pthread (or whatever option is needed)? I like
the idea of fully automating it, but that's a pretty big m4 file!

Anyway, I'd like to hear what the GCC folks think of adding
ax_pthread.m4 to the config/ directory.

(BTW, In the future, please omit diffs from generated files from your patch.)

-cary