[Openmp-dev] Two independent patches to the source code

[Peyton, Jonathan L]

Thanks for pointing that out Hal.
Here is an updated patch that changes that comment.

-- Johnny

-----Original Message-----
From: Hal Finkel [mailto:hfinkel at anl.gov] 
Sent: Monday, December 15, 2014 5:53 PM
To: Peyton, Jonathan L
Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: Re: [Openmp-dev] Two independent patches to the source code

----- Original Message -----
> From: "Jonathan L Peyton" <jonathan.l.peyton at intel.com>
> To: openmp-dev at dcs-maillist2.engr.illinois.edu
> Sent: Monday, December 15, 2014 5:31:33 PM
> Subject: [Openmp-dev] Two independent patches to the source code
> 
> We have received criticism for large, monolithic commits instead of 
> creating smaller independent patches. We have taken this criticism 
> into consideration and now have some small independent patches ready 
> for review.

Great, thanks!

> 
> 
> 
> 1) hier_barrier_fix.patch – This patch has a couple small fixes to the 
> hierarchical barrier, predominately used on the Intel® MIC 
> Architecture, in the context of teams and infinite blocktime. The 
> value of 0 for nesting level is used in the parallel region that 
> consists of the team masters, and hierarchical barrier did not take 
> that into consideration.
> 
> 
> 
> 2) balanced_affinity.patch – This patch enables the use of 
> KMP_AFFINITY=balanced on non-MIC Architectures. The restriction for 
> using balanced affinity on non-MIC architectures is it only works for 
> one-package machines.

I realize that this is not particular to this patch (which is just removing the #if KMP_MIC), but:

        case affinity_balanced:
        // Balanced works only for the case of a single package and uniform topology
        if( nPackages > 1 ) {
            if( __kmp_affinity_verbose || __kmp_affinity_warnings ) {
                KMP_WARNING( AffBalancedNotAvail, "KMP_AFFINITY" );
            }
            __kmp_affinity_type = affinity_none;
            return;
        } else if( __kmp_affinity_uniform_topology() ) {
            break;
        } else { // Non-uniform topology

            // Save the depth for further usage
            __kmp_aff_depth = depth;
            ...

It looks like the comment "Balanced works only for the case of a single package and uniform topology" should just read "Balanced works only for the case of a single package" because the non-uniform topology case is not only supported, but where all of the non-trivial logic actually is.

Thanks again,
Hal

> 
> 
> 
> To apply the patches:
> 
> Put them in the directory that contatins offload/, runtime/, etc.
> 
> >$ patch –p0 < hier_barrier_fix.patch
> 
> >$ patch –p0 < balanced_affinity.patch
> 
> 
> 
> Please leave feedback!
> 
> 
> 
> -- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: balanced_affinity.patch
Type: application/octet-stream
Size: 8929 bytes
Desc: balanced_affinity.patch
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20141217/7865d151/attachment.obj>