[Openmp-dev] Another four independent patches

[Hal Finkel]

----- Original Message -----
> From: "Jonathan L Peyton" <jonathan.l.peyton at intel.com>
> To: "Jonathan L Peyton" <jonathan.l.peyton at intel.com>, openmp-dev at dcs-maillist2.engr.illinois.edu
> Cc: "Brian E Bliss" <brian.e.bliss at intel.com>
> Sent: Monday, January 26, 2015 10:34:58 AM
> Subject: Re: [Openmp-dev] Another four independent patches 
> 
> It’s been nearly a week, is it ok to commit these?
> -- Johnny
> 
> From: openmp-dev-bounces at cs.uiuc.edu
> [mailto:openmp-dev-bounces at cs.uiuc.edu] On Behalf Of Peyton,
> Jonathan L
> Sent: Tuesday, January 20, 2015 4:53 PM
> To: openmp-dev at dcs-maillist2.engr.illinois.edu
> Cc: Bliss, Brian E
> Subject: [Openmp-dev] Another four independent patches
> 
> 
> 
> Descriptions of each patch (they are all independent):
> 
> 1) teams_construct_fix.patch -- Fixed implementation of the teams
> construct in case it contains parallel regions with different number
> of threads and/or has pre or post parallel region. Task teams are
> synchronized for each parallel and nesting levels are calculated
> correctly inside the barrier.

LGTM.

> 
> 2) group_affinity.patch -- This replaces #if KMP_OS_WINDOWS &&
> KMP_ARCH_X86_64 or any combination of those two options with the
> feature macro KMP_GROUP_AFFINITY. This also adds some new Windows
> processor groups trace messages and enables a cpuid leaf 4 check for
> non-MIC x86 architectures.

LGTM. This should really be multiple patches as far as I can tell (the CPUID change, the ifdef changes with some reformatting, and the new trace messages), please commit them separately. Also, we generally don't require pre-commit review for adding new useful debugging output.

> 
> 3) remove_svn_revision_date.patch – Although this seems like a lot of
> changes it is just removing the $Revision and $Date svn variables
> from the top of all the source files. Even though it touches
> numerous files, it should not interfere with any current
> development.

LGTM (Thanks!)

> 
> 4) unused_vars_removed.patch – This patch removes some unused
> variables (__kmp_ht_*) and changes__kmp_ncores and
> __kmp_nThreadsPerCore to static globals within kmp_affinity.cpp.
> 

Removing unused variables never requires pre-commit review. Making __kmp_nThreadsPerCore static, now that that's possible, falls into the same category. LGTM.

 -Hal

> 
> 
> To apply patches
> 
> $ patch –p0 < teams_construct_fix.patch
> 
> $ patch –p0 < group_affinity.patch
> 
> $ patch –p0 < unused_vars_removed.patch
> 
> $ patch –p0 < remove_svn_revision_date.patch
> 
> 
> 
> Since the remove_svn_revision_date.patch touches so many files at the
> very top, if you apply it before the others, you will inevitably get
> many Hunk #’s succeeded with offset messages for the subsequent
> patches. So I would apply it last.
> 
> 
> 
> -- Johnny
> 
> 
> _______________________________________________
> Openmp-dev mailing list
> Openmp-dev at dcs-maillist2.engr.illinois.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev
> 

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