<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 24, 2012, at 3:05 AM, Alexey Bataev <<a href="mailto:a.bataev@gmx.com">a.bataev@gmx.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span style="font-family:Verdana"><span style="font-size:12px">Doug,<br>
I've fixed the patch. Please, take a look. <br></span></span></blockquote><div><br></div>I went ahead and committed a tweaked version of this patch as r172509, thanks! A few nits (that I've fixed):</div><div><br></div><div><div>Index: include/clang/Driver/Options.td</div><div>===================================================================</div><div>--- include/clang/Driver/Options.td<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 169213)</div><div>+++ include/clang/Driver/Options.td<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</div><div>@@ -598,7 +598,8 @@</div><div> def fobjc_sender_dependent_dispatch : Flag<["-"], "fobjc-sender-dependent-dispatch">, Group<f_Group>;</div><div> def fobjc : Flag<["-"], "fobjc">, Group<f_Group>;</div><div> def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group<f_Group>;</div><div>-def fopenmp : Flag<["-"], "fopenmp">, Group<f_Group>;</div><div>+def fopenmp : Flag<["-"], "fopenmp">, Group<f_Group>, Flags<[CC1Option]>;</div><div>+def fno_openmp : Flag<["-"], "fno-openmp">, Group<f_Group>;</div><div><br></div><div>-fno-openmp is no longer used.</div><div><br></div><div><div>+ // OpenMP definition</div><div>+ if (LangOpts.OpenMP) {</div><div>+ // OpenMP 2.2: In implementations that supports a preprocessor, the _OPENMP macro name</div><div>+ // is defined to have the decimal value yyyymm where yyyy and mm are the year and</div><div>+ // the month designations of the version of the OpenMP API that the implementation support.</div><div>+ Builder.defineMacro("_OPENMP", "201107");</div><div>+ }</div><div><br></div><div>Sources should always be wrapped at 80 columns.</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><div><br></div></div><blockquote type="cite"><span style="font-family:Verdana"><span style="font-size:12px">
Best regards,<br>
<br>
Alexey Bataev<br>
=============<br>
Software Engineer<br>
Intel Compiler Team<br>
<br><p style="margin:0px; padding:0px;">
</p>
<blockquote style="border-left: 1px solid #CCC; padding-left: 5px; margin-left: 5px; margin-bottom: 0px; margin-top: 0px; margin-right: 0px;" type="cite"><div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px">----- Original Message -----</span></span></div><div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px">From: Douglas Gregor</span></span></div><div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px">Sent: 12/21/12 10:01 PM</span></span></div><div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px">To: Alexey Bataev</span></span></div><div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px">Subject: Re: [cfe-commits] [PATCH] First OpenMP patch</span></span></div>
<br>
<div>
<br>
<div>
<div>
On Dec 20, 2012, at 8:47 PM, Alexey Bataev <<a href="mailto:a.bataev@gmx.com">a.bataev@gmx.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px">Hi Doug,<br>
Thank you for review. Here are my comments.<br>
1. include/clang/Basic/DiagnosticDriverKinds.td (no changes) - Agree, my bad.<br>
2. docs/OpenMP.rst - Ok, I'll remove it.<br>
3. lib/Frontend/InitPreprocessor.cpp (comment) - Agree. As to version number, I think we can remove it. It seems the section numbers are stable.</span></span></blockquote>
<div>
</div>
Okay.</div>
<div>
<br>
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px">4. lib/Driver/Tools.cpp and include/clang/Basic/DiagnosticGroups.td (source-uses-openmp warning group) - Do we really need to remove this warning group? It is reserved for future use, I plan to emit a warning on the very first OpenMP pragma if no -fopenmp compiler option was specified. This warning (and maybe some others) is planned to be the part of this warning group, so the user could turn on/off the warning.</span></span></blockquote>
<div>
</div>
<div>
This warning doesn't make sense until -fopenmp actually works. We should not change the behavior of the compiler w.r.t. OpenMP for users until we flip the switch to make -fopenmp turn on our complete, working implementation.</div>
<div>
</div>
<div>
Additionally, the only reason to add a placeholder diagnostic group is for backward compatibility, so we recognize that warning group name on the command line. For new warning groups, they should be added with the first warning in that group.</div>
<br>
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px">5. test/Driver/openmp-options.c - if warning group won't be removed, we need to keep this test. As to OpenMP/<new-test>, I'll add it.</span></span></blockquote>
<div>
</div>
Okay. test/Derived/openmp-options.c can come back when it makes sense. We're not there yet.</div>
<div>
</div>
<div>
- Doug</div>
<div>
<br>
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px">Best regards,<br>
Alexey Bataev<br>
============= </span></span>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Software Engineer</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Intel Compiler Team</span></span></div>
<br><p style="margin:0px; padding:0px; margin:0px; padding:0px;">
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></p>
<blockquote style="border-left: 1px solid #CCC; padding-left: 5px; margin-left: 5px; margin-bottom: 0px; margin-top: 0px; margin-right: 0px;" type="cite">
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">----- Original Message -----</span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">From: Douglas Gregor</span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Sent: 12/20/12 11:40 PM</span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">To: Alexey Bataev</span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Subject: Re: [cfe-commits] [PATCH] First OpenMP patch</span></span> </span></span></div>
<br>
<div>
<br>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">On Dec 20, 2012, at 1:43 AM, Alexey Bataev <<a href="mailto:a.bataev@gmx.com">a.bataev@gmx.com</a>> wrote:</span></span></div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Hello guys,<br>
I agree with all your comments. So I fixed the initial patch. See attach.</span></span> </span></span></blockquote>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<span style="font-family:Verdana"><span style="font-size:12px">Thanks. A few more comments:</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Index: include/clang/Basic/DiagnosticDriverKinds.td</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">===================================================================</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">--- include/clang/Basic/DiagnosticDriverKinds.td<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 169213)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+++ include/clang/Basic/DiagnosticDriverKinds.td<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">@@ -146,4 +146,4 @@</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> "analyzer-config option '%0' has a key but no value">;</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> def err_analyzer_config_multiple_values : Error<</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> "analyzer-config option '%0' should contain only one '='">;</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">-}</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+}</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">This isn't actually changing anything.</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Index: docs/OpenMP.rst</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">===================================================================</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">--- docs/OpenMP.rst<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+++ docs/OpenMP.rst<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 0)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">@@ -0,0 +1,21 @@</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">We don't need this now.</span></span></div>
</div>
</div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Index: lib/Frontend/InitPreprocessor.cpp</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">===================================================================</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">--- lib/Frontend/InitPreprocessor.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 169213)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+++ lib/Frontend/InitPreprocessor.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">@@ -641,6 +641,13 @@</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> "__attribute__((objc_ownership(none)))");</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> }</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ // OpenMP definition</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ if (LangOpts.OpenMP) {</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ // Define _OPENMP macro with value 201107 for OpenMP v. 3.1 </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ // according to p. 2.2 of OpenMP API specification</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ Builder.defineMacro("_OPENMP", "201107");</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+ }</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">We're going to end up with a lot of OpenCP specification citations, so let's settle on a form now. I suggest:</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">// OpenMPv3.1 2.2:</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">// <span style="font-size: 10pt; font-family: TimesNewRoman; ">In implementations that support a preprocessor, the </span><span style="font-size: 10pt; font-family: Courier; font-weight: 700; ">_OPENMP </span><span style="font-size: 10pt; font-family: TimesNewRoman; ">macro name is defined to have </span> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-size: 10pt; font-family: TimesNewRoman; ">// the decimal value </span><span style="font-size: 10pt; font-family: 'TimesNewRoman,Italic'; ">yyyymm </span><span style="font-size: 10pt; font-family: TimesNewRoman; ">where </span><span style="font-size: 10pt; font-family: 'TimesNewRoman,Italic'; ">yyyy </span><span style="font-size: 10pt; font-family: TimesNewRoman; ">and </span><span style="font-size: 10pt; font-family: 'TimesNewRoman,Italic'; ">mm </span><span style="font-size: 10pt; font-family: TimesNewRoman; ">are the year and month designations of the </span> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-size: 10pt; font-family: TimesNewRoman; ">// version of the OpenMP API that the implementation supports.</span> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">which is similar to what we do for C/C++. If section numbers tend to be stable, we can drop the v3.1. I don't know OpenMP well enough to guess.</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">Index: lib/Driver/Tools.cpp</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">===================================================================</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">--- lib/Driver/Tools.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 169213)</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+++ lib/Driver/Tools.cpp<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">@@ -2510,6 +2510,15 @@</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> </span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> Args.AddLastArg(CmdArgs, options::OPT_pthread);</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> </span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ // When both -fopenmp and -fno-openmp options are present,</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ // later one wins.</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ if (Arg *A = Args.getLastArg(options::OPT_fopenmp,</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ options::OPT_fno_openmp)) {</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ // Do not pass -fopenmp from driver to frontend for now</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ if (A->getOption().matches(options::OPT_fno_openmp)) {</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ CmdArgs.push_back("-Wno-source-uses-openmp");</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ }</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">+ }</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> </span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> // -stack-protector=0 is default.</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;"> unsigned StackProtectorLevel = 0;</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"><font face="TimesNewRoman"><span style="font-size: 13px;">Since there are no longer any warnings under this diagnostic group, this code should be removed.</span></font> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
</div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
</div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Index: include/clang/Basic/DiagnosticGroups.td</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">===================================================================</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">--- include/clang/Basic/DiagnosticGroups.td<span class="Apple-tab-span" style="white-space:pre"> </span>(revision 169213)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+++ include/clang/Basic/DiagnosticGroups.td<span class="Apple-tab-span" style="white-space:pre"> </span>(working copy)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">@@ -407,6 +407,9 @@</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> ThreadSafetyAnalysis,</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> ThreadSafetyPrecise]>;</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+// OpenMP warnings</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+def SourceUsesOpenMP : DiagGroup<"source-uses-openmp">;</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> // Note that putting warnings in -Wall will not disable them by default. If a</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> // warning should be active _only_ when -Wall is passed in, mark it as</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> // DefaultIgnore in addition to putting it here.</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">We don't need this diagnostic group now.</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Index: test/Driver/openmp-options.c</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">===================================================================</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">--- test/Driver/openmp-options.c<span class="Apple-tab-span" style="white-space: pre; "> </span>(revision 0)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">+++ test/Driver/openmp-options.c<span class="Apple-tab-span" style="white-space: pre; "> </span>(revision 0)</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">I think this test should go away for now, since we don't want to have -Wno-source-uses-openmp.</span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">Instead, we should have a test in a new test/OpenMP directory that runs clang -cc1 -fopenmp and then uses an #if/#error to make sure _OPENMP is being set correctly.</span></span></div>
</div>
</div>
</div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></div>
<div>
<span style="font-family:Verdana"><span style="font-size:12px">- Doug</span></span></div>
<div>
<br>
<blockquote type="cite">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Best regards,<br>
Alexey.</span></span></span></span><br>
<br><p style="margin:0px; padding:0px; margin:0px; padding:0px; margin:0px; padding:0px;">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"> </span></span></span></span></p>
<blockquote style="border-left: 1px solid #CCC; padding-left: 5px; margin-left: 5px; margin-bottom: 0px; margin-top: 0px; margin-right: 0px;" type="cite">
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">----- Original Message -----</span></span> </span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">From: Richard Smith</span></span> </span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Sent: 12/20/12 01:12 AM</span></span> </span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">To: Dmitri Gribenko</span></span> </span></span> </span></span></div>
<div style="margin: 0px; padding: 0px; ">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px">Subject: Re: [cfe-commits] [PATCH] First OpenMP patch</span></span> </span></span> </span></span></div>
<br>
<div>
<div>
<pre style="white-space: pre-wrap; word-wrap: break-word; font-size:11;white-space: pre-wrap; word-wrap: break-word; font-size:11;white-space: pre-wrap; word-wrap: break-word; font-size:11;pre">
<span style="font-family:Verdana"><span style="font-size:12px">
<span style="font-family:Verdana"><span style="font-size:12px">
On Wed, Dec 19, 2012 at 1:09 PM, Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>> wrote:
> On Wed, Dec 19, 2012 at 10:53 PM, Douglas Gregor <<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>> wrote:
>>
>> On Dec 19, 2012, at 3:30 AM, Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>> wrote:
>>
>>> On Wed, Dec 19, 2012 at 1:02 PM, Alexey Bataev <<a href="mailto:a.bataev@gmx.com">a.bataev@gmx.com</a>> wrote:
>>>> Dmitry, thanks for a good advice about :option:. Fixed patch is in the
>>>> attach.
>>>
>>> LGTM. I think that there is a consensus about command line flags and
>>> warnings, so this can go in.
>>
>> Sorry to come late to the party, but I think this is the wrong approach for our users. Our current behavior is that we simply drop "-fopenmp". If we change that behavior to
>>
>> warning: Experimental OpenMP support enabled
>>
>> how have we helped the user? At best, it does nothing and they ignore the warning. At worst, it opens up a ton of questions: should I depend on the experimental implementation? What does it actually do? What was it doing before? How do I shut this warning off? This is especially important to consider for options like -fopenmp that currently exist for GNU compatibility. Changing the behavior in any way other than actually implementing that functionality is not a net win for users.
>>
>> All we need for OpenMP at this stage is a new LangOpt for OpenMP (which is part of the patch) and a -cc1 option "-fopenmp" that turns on that LangOpt. It also makes sense to define _OPENMP when that LangOpt is true, so that we can write a test for -fopenmp.
>>
>> It is very important that "-fopenmp" still be dropped by the driver. The -fopenmp at the -cc1 level will be separate, and used for our testing. Once OpenMP is actually usable, we can wire the driver's -fopenmp up to the -cc1 -fopenmp.
>
> That's a good point. But in order to test the implementation on some
> real OpenMP programs easily, we might still want to introduce a
> frontend option, just a different one -- like '-fexperimental-openmp'.
> We can also drop the warning then.
For testing, we can use -Xclang -fopenmp</span></span></span></span></pre>
</div>
</div>
</blockquote><p style="margin:0px; padding:0px; margin:0px; padding:0px; margin:0px; padding:0px;">
<span style="font-family:Verdana"><span style="font-size:12px"><span style="font-family:Verdana"><span style="font-size:12px"> </span></span></span></span></p>
<br>
<span style="font-family:Verdana"><span style="font-size:12px"><span><patch.p></span> </span></span></blockquote>
</div>
</div>
</blockquote><p style="margin:0px; padding:0px; margin:0px; padding:0px;">
<span style="font-family:Verdana"><span style="font-size:12px"> </span></span></p>
</blockquote>
</div>
</div>
</blockquote><p style="margin:0px; padding:0px;">
</p>
<br>
</span></span>
<span><patch.p></span></blockquote></div><br></body></html>