[Openmp-commits] TLS for POWER

[Hal Finkel]

[+Johnny, Andrey]

Any objections to this direction?

 -Hal

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Alexandre Eichenberger" <alexe at us.ibm.com>
> Cc: openmp-commits at dcs-maillist2.engr.illinois.edu
> Sent: Monday, June 22, 2015 8:38:28 PM
> Subject: Re: [Openmp-commits] TLS for POWER
> 
> Hi Alexandre,
> 
> Thanks!
> 
> Please also post the Clang patch for review on the cfe list.
> 
>  -Hal
> 
> ----- Original Message -----
> > From: "Alexandre Eichenberger" <alexe at us.ibm.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: openmp-commits at dcs-maillist2.engr.illinois.edu
> > Sent: Monday, June 22, 2015 4:17:22 PM
> > Subject: Re: [Openmp-commits] TLS for POWER
> > 
> > Hal, Brian
> > 
> > below is the revised patch.  I defined a new macro
> > KMP_THREADPRIVATE_TLS
> > that is currently set only for PPC64, and other architectures/OS
> > can
> > be
> > added in that macro definition.
> > 
> > I added comments when defining functions, as requested.
> > 
> > I could not add "static" for the kmp_threadprivate.c function as it
> > is
> > defined there, but used in kmp_runtime.c
> > 
> > Let me know of any additional suggestion you or fellow reviewers
> > may
> > have
> > 
> > Alexandre
> > 
> > (See attached file: threadprivate-patch2)
> > 
> > 
> > 
> > From:	Hal Finkel <hfinkel at anl.gov>
> > To:	Alexandre Eichenberger/Watson/IBM at IBMUS
> > Cc:	<openmp-commits at dcs-maillist2.engr.illinois.edu>
> > Date:	06/18/2015 12:50 PM
> > Subject:	Re: [Openmp-commits] TLS for POWER
> > 
> > 
> > 
> > Hi Alexandre,
> > 
> > First, thanks for working on this! I think that we should
> > definitely
> > enable
> > this mode of supporting thread locals if possible.
> > 
> > As an organizational note, I think it would be better to add a new
> > feature
> > macro for using TLS in this way (currently enabled only for PPC64).
> > As you
> > point out, other architectures will likely also want to do this,
> > and
> > we
> > should make this straightforward.
> > 
> > +#if KMP_ARCH_PPC64
> > +void kmpc_threadprivate_call_unprocessed_constructors();
> > +#endif
> > 
> > This function could be static, right?
> > 
> > +       list. This is because each thread maintain a private
> > variable
> > 
> > maintain -> maintains
> > 
> > +void
> > +kmpc_threadprivate_call_unprocessed_constructors()
> > 
> > I don't like that the comment explaining what this does is embedded
> > in some
> > other function (above). Please at least comment on where the
> > comment
> > is
> > explaining what this does.
> > 
> > +        // one or more register occured since the last parallel
> > loop
> > 
> > register -> registrations
> > 
> >  -Hal
> > 
> > ----- Original Message -----
> > > From: "Alexandre Eichenberger" <alexe at us.ibm.com>
> > > To: openmp-commits at dcs-maillist2.engr.illinois.edu
> > > Sent: Friday, June 12, 2015 5:51:27 PM
> > > Subject: [Openmp-commits] TLS for POWER
> > >
> > >
> > > I would like to request switching the threadprivate scheme to TLS
> > > (Thread
> > > Level Storage) for POWER before we get legacy code out there.
> > > Individual
> > > measurements has shown TLS to have about 10x lower overhead
> > > (number
> > > may
> > > change depending to the TLS scheme). TLS is also used by other
> > > compilers
> > > (e.g. XL) and thus provide better interoperability.
> > >
> > > Approach is relatively simple: compiler still generate a call to
> > > register
> > > the constructors and the destructors, but the functions calling
> > > the
> > > constructor/destructors have been changed a bit so that the
> > > compiler
> > > call
> > > the constructor on the TLS version of the object (not the global
> > > one,
> > > as it
> > > currently is in KMP). Then, prior to starting a parallel
> > > function,
> > > the
> > > workers check if there are no new constructors/destructors that
> > > have
> > > been
> > > registered; if there are, they will call the constructors prior
> > > to
> > > starting
> > > the parallel region. Destructors are called when killing a worker
> > > thread.
> > >
> > > KMP code change have been done (exclusively under #def for power
> > > architecture), LLVM changes have been done, code has been tested.
> > > Commit
> > > will need to be coordinated with the LLVM changes, which must go
> > > at
> > > the
> > > same time.
> > >
> > > If there is interest, and no code legacy issue, this patch could
> > > also
> > > be
> > > applied for other architectures that have TLS support.
> > >
> > > Alexandre
> > >
> > >
> > > (See attached file: tls_patch)
> > > _______________________________________________
> > > Openmp-commits mailing list
> > > Openmp-commits at dcs-maillist2.engr.illinois.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/openmp-commits
> > >
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> Openmp-commits mailing list
> Openmp-commits at dcs-maillist2.engr.illinois.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/openmp-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory