[Openmp-dev] Three small patches

[Peyton, Jonathan L]

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.

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