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