Three small patches

Hello,

Here are three very small patches:

  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.

  2. affinity_comment.patch: comment for affinity balanced improved as suggested in LLVM review.

  3. placement.patch: corrected dn_successors placement

Apologies for not using whatever mechanisms are in place for submitting patches for review. Johnny is on leave, and I know he was working on a better way to get our changes out, so please bear with me as I try to keep us from getting too far behind on these.

Thanks!

Terry

build_fixes.patch (2.23 KB)

affinity_comment.patch (611 Bytes)

placement.patch (702 Bytes)

From: "Terry L Wilmarth" <terry.l.wilmarth@intel.com>
To: openmp-dev@dcs-maillist2.engr.illinois.edu
Sent: Tuesday, March 3, 2015 11:16:49 AM
Subject: [Openmp-dev] Three small patches

Hello,

Here are three very small patches:

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.

"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).

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).

3) placement.patch: corrected dn_successors placement

And also adds dn_routine? What was incorrect about the previous definition?

-Hal

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.)

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.

From: "Terry L Wilmarth" <terry.l.wilmarth@intel.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: openmp-dev@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

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

From: "Jonathan L Peyton" <jonathan.l.peyton@intel.com>
To: "Hal Finkel" <hfinkel@anl.gov>, "Terry L Wilmarth" <terry.l.wilmarth@intel.com>
Cc: openmp-dev@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