[Openmp-dev] Three small patches

[Hal Finkel]

----- Original Message -----
> From: "Terry L Wilmarth" <terry.l.wilmarth at intel.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
> Sent: Wednesday, March 4, 2015 12:29:31 PM
> Subject: RE: [Openmp-dev] Three small patches
> 
> Thanks Hal, a couple comments below.
> 
> > 1) build_fixes.patch: removes -fstack-protector flags from oss/llvm
> > builds; fixes an oversight in AsmFlags for builds using icc on
> > Windows; adds KMP_NESTED_HOT_TEAMS definition.
> 
> "adds KMP_NESTED_HOT_TEAMS definition." -- this part LGTM, please
> commit it separately.
> [Terry L Wilmarth] Thanks, we will commit this part.
> 
> "removes -fstack-protector flags from oss/llvm builds" -- but the
> patch does not appear to do this. It looks like it is moving from MS
> build flag definitions around (and I'm less fond of the new
> formatting).
> 
> [Terry L Wilmarth] My error in leaving this in the description --
> when I made the patch, I found that the -fstack_protector flags had
> already been removed earlier, so perhaps Johnny had already applied
> that part of the change.
> 
> [Terry L Wilmarth] For the AsmFlags changes, do you have a suggestion
> for improving? (I've fixed that stray tab in the formatting that I
> missed in the patch.)

It is a bit hard to tell without context, what is the change actually doing?

 -Hal

> 
> > 
> > 2) affinity_comment.patch: comment for affinity balanced improved
> > as
> > suggested in LLVM review.
> 
> LGTM (you don't need pre-commit review for comment improvements).
> 
> [Terry L Wilmarth] Thanks!
> 
> > 
> > 3) placement.patch: corrected dn_successors placement
> 
> And also adds dn_routine? What was incorrect about the previous
> definition?
> 
> [Terry L Wilmarth] We are going to hold off on this patch for now, as
> it looks like it may be better coupled with a larger patch it is
> related to.
> 
> 

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