<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: arial,helvetica,sans-serif; font-size: 10pt; color: #000000'><br><hr id="zwchr"><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><b>From: </b>"Terry L Wilmarth" <terry.l.wilmarth@intel.com><br><b>To: </b>openmp-dev@dcs-maillist2.engr.illinois.edu<br><b>Sent: </b>Thursday, March 26, 2015 12:32:46 PM<br><b>Subject: </b>[Openmp-dev] Two patches<br><br>
<style><!--
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Gisha;
panose-1:2 11 5 2 4 2 4 2 2 3;}
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Gisha",sans-serif;
color:windowtext;
font-weight:normal;
font-style:normal;
text-decoration:none none;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
@list l0
{mso-list-id:1164513863;
mso-list-type:hybrid;
mso-list-template-ids:-265284680 67698703 67698713 67698715 67698703 67698713 67698715 67698703 67698713 67698715;}
@list l0:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level2
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level3
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level4
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level5
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level6
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
@list l0:level7
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level8
{mso-level-number-format:alpha-lower;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;}
@list l0:level9
{mso-level-number-format:roman-lower;
mso-level-tab-stop:none;
mso-level-number-position:right;
text-indent:-9.0pt;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style>
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">Hello,</span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"> </span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">Here are 2 more patches:</span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"> </span></p>
<p class="MsoListParagraph" style="text-indent: -0.25in;"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"><span style="">1.<span style="font: 7pt "Times New Roman";">
</span></span></span><span id="DWT2431" style="font-size: 10pt; font-family: "Gisha",sans-serif;">hier1.patch: This short patch eliminates writing to the depth field of the machine_hierarchy data structure. The write was unnecessary, as the depth can simply be
pulled out of the data structure to a local variable, and then modified as needed. This also allowed the removal of the base_depth field, because now the depth field is the base depth, and does not need to be modified.</span></p></div></blockquote><br>LGTM, but we need more comments here. First, please add a comment near where skipPerLevel is declared noting what the field means (looks like it is numPerLevel, but cumulative over all parent levels?). Regarding the code you're fixing, please add a comment explaining what is happening. It looks like you're increasing the effective barrier scheduling granularity if you have more participating threads than <span style="white-space: pre;"></span><span id="DWT2431" style="font-size: 10pt; font-family: "Gisha",sans-serif;">machine_hierarchy units at that granularity, right?</span><br><br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div class="WordSection1"><p class="MsoListParagraph" style="text-indent: -0.25in;"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"></span></p>
<p class="MsoListParagraph" style="text-indent: -0.25in;"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"><span style="">2.<span style="font: 7pt "Times New Roman";">
</span></span></span><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">safe_api.patch: This patch is large, but very simple. This change replaces banned C API calls with safe calls to meet the SDL guidelines. Most calls have their safe
alternatives on Windows, but we just define macros for other platforms.</span></p>
<p class="MsoListParagraph" style="margin-left: 1in; text-indent: -0.25in;">
<span style="font-size: 10pt; font-family: "Gisha",sans-serif;"><span style="">a.<span style="font: 7pt "Times New Roman";">
</span></span></span><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">kmp_safe_c_api.h defines macros that replace the banned calls</span></p>
<p class="MsoListParagraph" style="margin-left: 1in; text-indent: -0.25in;">
<span style="font-size: 10pt; font-family: "Gisha",sans-serif;"><span style="">b.<span style="font: 7pt "Times New Roman";">
</span></span></span><span id="DWT2432" style="font-size: 10pt; font-family: "Gisha",sans-serif;">APIs for non-windows platforms will be filled in later</span></p></div></blockquote><br>Okay. Two questions:<br><br>+// _malloca was suggested, but it is not a drop-in replacement for _alloca<br>+# define KMP_ALLOCA _alloca<br><br>Is the only difference the fact that the size is capped at _ALLOCA_S_THRESHOLD? Would you hit this limit? Are there performance implications to making the change regardless?<br><br>Index: runtime/src/kmp_barrier.cpp<br>===================================================================<br>--- runtime/src/kmp_barrier.cpp (revision 231388)<br>+++ runtime/src/kmp_barrier.cpp (working copy)<br>@@ -32,7 +32,7 @@<br> #else<br> #define ngo_load(src) ((void)0)<br> #define ngo_store_icvs(dst, src) copy_icvs((dst), (src))<br>-#define ngo_store_go(dst, src) memcpy((dst), (src), CACHE_LINE)<br>+#define ngo_store_go(dst, src) KMP_MEMCPY((dst), (src), CACHE_LINE)<br> #define ngo_sync() ((void)0)<br> #endif /* KMP_MIC && USE_NGO_STORES */<br><br>The other changes look fine, but this could have performance implications, no? The compiler can optimize away the memcpy, but likely not the memcpy_s, and the associated validation is likely expensive. Should we avoid this here?<br><br>Thanks again,<br>Hal<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; color: rgb(0, 0, 0); font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica,Arial,sans-serif; font-size: 12pt;"><div class="WordSection1"><p class="MsoListParagraph" style="margin-left: 1in; text-indent: -0.25in;"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"></span></p>
<p class="MsoListParagraph" style="margin-left: 1in;"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"> </span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">Thanks!</span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;">Terry</span></p>
<p class="MsoNormal"><span style="font-size: 10pt; font-family: "Gisha",sans-serif;"> </span></p>
<p class="MsoNormal" style=""><span style="font-size: 8pt; font-family: "Gisha",sans-serif;">--<br>
Terry L. Wilmarth<br>
terry.l.wilmarth@intel.com 217/403-4251<br>
Intel/SSG/DPD/TCAR/RAD/Threading Runtimes </span></p>
<p class="MsoNormal"> </p>
</div>
<br>_______________________________________________<br>Openmp-dev mailing list<br>Openmp-dev@dcs-maillist2.engr.illinois.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev<br></blockquote><br><br><br>-- <br><div><span name="x"></span>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<span name="x"></span><br></div></div></body></html>