[Openmp-dev] Three small patches

[Hal Finkel]

----- Original Message -----
> From: "Jonathan L Peyton" <jonathan.l.peyton at intel.com>
> To: "Hal Finkel" <hfinkel at anl.gov>, "Terry L Wilmarth" <terry.l.wilmarth at intel.com>
> Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
> Sent: Wednesday, March 4, 2015 3:26:57 PM
> Subject: RE: [Openmp-dev] Three small patches
> 
> Hal,
> 
> It is just moving Windows-specific flags and definitions under the
> WINDOWS guard in CMake.  -safeseh, -coff are flags that only exist
> on the Intel Compiler on Microsoft Windows, and the definitions are
> only relevant on Microsoft Windows.
> 

Okay, sure. LGTM.

 -Hal

> Sorry for the confusion.
> 
> -- Johnny
> 
> -----Original Message-----
> From: openmp-dev-bounces at cs.uiuc.edu
> [mailto:openmp-dev-bounces at cs.uiuc.edu] On Behalf Of Hal Finkel
> Sent: Wednesday, March 4, 2015 12:38 PM
> To: Wilmarth, Terry L
> Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
> Subject: Re: [Openmp-dev] Three small patches
> 
> ----- 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
> _______________________________________________
> 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