public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [rfc] gcc trunk - libgomp thread affinity for freebsd
@ 2015-05-06 23:29 Adrian Chadd
  2015-05-07  7:57 ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Adrian Chadd @ 2015-05-06 23:29 UTC (permalink / raw)
  To: gcc-patches

Hi,

This patch implements basic top level thread affinity support for
freebsd. It doesn't yet implement thread affinity support for
core/socket grouping yet; I'm working on a library to extract that out
to userland and plan on teaching libgomp about it at a later stage.

https://people.freebsd.org/~adrian/gcc/20150506-gcc-trunk-libgomp-1.diff

I'd appreciate feedback/review.

Thanks!


-adrian

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [rfc] gcc trunk - libgomp thread affinity for freebsd
  2015-05-06 23:29 [rfc] gcc trunk - libgomp thread affinity for freebsd Adrian Chadd
@ 2015-05-07  7:57 ` Jakub Jelinek
  2015-05-07 19:20   ` Adrian Chadd
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2015-05-07  7:57 UTC (permalink / raw)
  To: Adrian Chadd; +Cc: gcc-patches

On Wed, May 06, 2015 at 04:29:02PM -0700, Adrian Chadd wrote:
> This patch implements basic top level thread affinity support for
> freebsd. It doesn't yet implement thread affinity support for
> core/socket grouping yet; I'm working on a library to extract that out
> to userland and plan on teaching libgomp about it at a later stage.
> 
> https://people.freebsd.org/~adrian/gcc/20150506-gcc-trunk-libgomp-1.diff
> 
> I'd appreciate feedback/review.

In affinity.c, that sounds like way too much code duplication, besides
slightly different includes (but, seems it is only added ones, not removed),
the only differences I see are:
1) you are assuming HAVE_PTHREAD_AFFINITY_NP is defined, is that really
needed?  The configure test should pass and define this macro if you have
it
2) cpu_set_t vs. cpuset_t
3) assuming CPU_ALLOC_SIZE is not defined and so aren't CPU_*_S
4) gomp_cpuset_popcount vs. CPU_COUNT
5) gomp_affinity_init_level is different

1) and 3) can be IMHO kept as is, for 2)/4) you could just add
a short config/freebsd/affinity.c wrapper that includes right headers,
defines a few macros and finally #include "../linux/affinity.c"
For 5), guess that function could be moved into some header
(affinity-init.h) and you could have a different version in config/linux and
config/freebsd.  As you add your own proc.c, I guess for 4) also
just defining your own gomp_cpuset_popcount in there would work too.

	Jakub

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [rfc] gcc trunk - libgomp thread affinity for freebsd
  2015-05-07  7:57 ` Jakub Jelinek
@ 2015-05-07 19:20   ` Adrian Chadd
  0 siblings, 0 replies; 3+ messages in thread
From: Adrian Chadd @ 2015-05-07 19:20 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 7 May 2015 at 00:57, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 06, 2015 at 04:29:02PM -0700, Adrian Chadd wrote:
>> This patch implements basic top level thread affinity support for
>> freebsd. It doesn't yet implement thread affinity support for
>> core/socket grouping yet; I'm working on a library to extract that out
>> to userland and plan on teaching libgomp about it at a later stage.
>>
>> https://people.freebsd.org/~adrian/gcc/20150506-gcc-trunk-libgomp-1.diff
>>
>> I'd appreciate feedback/review.
>
> In affinity.c, that sounds like way too much code duplication, besides
> slightly different includes (but, seems it is only added ones, not removed),
> the only differences I see are:
> 1) you are assuming HAVE_PTHREAD_AFFINITY_NP is defined, is that really
> needed?  The configure test should pass and define this macro if you have
> it
> 2) cpu_set_t vs. cpuset_t
> 3) assuming CPU_ALLOC_SIZE is not defined and so aren't CPU_*_S
> 4) gomp_cpuset_popcount vs. CPU_COUNT
> 5) gomp_affinity_init_level is different
>
> 1) and 3) can be IMHO kept as is, for 2)/4) you could just add
> a short config/freebsd/affinity.c wrapper that includes right headers,
> defines a few macros and finally #include "../linux/affinity.c"
> For 5), guess that function could be moved into some header
> (affinity-init.h) and you could have a different version in config/linux and
> config/freebsd.  As you add your own proc.c, I guess for 4) also
> just defining your own gomp_cpuset_popcount in there would work too.

Yes,, a lot of it overlaps with the Linux code. I bet a lot of this
stuff can be refactored out into a common affinity routine where the
manipulation routines are defined per-platform and we implement the
stubs in both linux and freebsd. I'm sure other BSD's could benefit
from supporting thread pinning. (And I'm sure non-*NIXes could as
well.)

I don't mind attempting to take a stab at it, but is it something we
could do post initial commit? I'd like to get something useful into
the tree so people can immediately benefit from it and then we spin a
few cycles together figuring out how to do this in a more platform
independent way.

Thanks,


-adrian

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-07 19:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-06 23:29 [rfc] gcc trunk - libgomp thread affinity for freebsd Adrian Chadd
2015-05-07  7:57 ` Jakub Jelinek
2015-05-07 19:20   ` Adrian Chadd

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).