<div dir="ltr"><div class="gmail_quote">On Thu, May 28, 2015 at 2:46 PM Andrey Bokhanko <<a href="mailto:andreybokhanko@gmail.com">andreybokhanko@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Chandler,<br>
<br>
Issue #1 -- not an issue at all. Personally, I *welcome* your fixes,<br>
and it's logical they are coming from you -- as apart of users you<br>
mentioned, there are zero documented users who *rely* on clang linking<br>
libgomp and ignoring all OMP pragmas.<br>
<br>
As for lack of code review, I trust your judgment and experience here<br>
(and still (!) ready to learn from you... -- remember?) and more than<br>
trust your driver fu.<br>
<br>
Issue #2 -- yes, I'm concerned. This seriously affects "the shortest<br>
path" user interface to OMP implementation and runtime library. I<br>
believe it warrants at least a discussion and some kind of consent in<br>
the community. Obviously, involving code owners of said OMP<br>
implementations would be reasonable.<br></blockquote><div><br></div><div>I think that trunk should continue working for existing users during that discussion.</div><div><br></div><div>I also think the discussion is on-going as the OpenMP runtime issues are being addressed. I don't any particular problem with the progress there, I just think that the default flip should not happen until either:</div><div><br></div><div>a) The work completes and it is clearly working well for users to get both Clang and the new runtime and use them together, or</div><div><br></div><div>b) There is some widespread community desire to *force* the default to change and break existing users until the pieces are fit back together with the new runtime.</div><div><br></div><div>I don't like (b) and would argue against it, but I also haven't seen anyone who really does like (b).</div><div><br></div><div><br></div><div>I don't believe this meaningfully impacts the cost of users getting at the OMP implementation -- before Alexey's patch, they had to pass -fopenmp=libiomp5, now they still do. If anything, it is still easier as they can just have CMake remap the default.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> The default flip actually regressed a very large number of my users. Regressing users is *not* OK. Given the lack of response from the author, I think I was entirely justified returning the default to its previous state.<br>
<br>
That's surprising. We discussed this with Alexey and the very next day<br>
he responded with a fix that (as we hoped) resolves your concerns and<br>
gives an easy way to configure clang the way you and your users like.<br>
You replied to this fix with:<br>
<br>
> Yea, i think Daniel's patch was a closer start. Anyways, thanks for taking<br>
> a stab at this.<br>
<br>
(from <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9875&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=W_yTuvMV5wT89XMixwAbtS9I3v_XE33v25ty_tWjIRI&s=tk0O55WvCpnnc5Tuk8ZGCnSEaJ-Y8ZU9-N_UKMKVhUE&e=" target="_blank">http://reviews.llvm.org/D9875</a>)<br>
<br>
I interpreted it as "I'm not 100% happy but that's OK". Is my<br>
interpretation wrong?<br></blockquote><div><br></div><div>This only addressed one of the several complaints I raised in the review thread. And the review thread itself was never updated.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I *really* don't want us to become two boys shouting "it is libgomp!<br>
flip" and "no, it is libiomp! flip" to each other and flipping the<br>
switch ad infinitum. But I frankly fail to understand your logic...<br></blockquote><div><br></div><div>I don't think that's going to happen -- I think the patch to flip the default just went in too soon.</div><div><br></div><div>FTR, given the progress on the CMake and lit side of things, I wouldn't expect this to even be risky for 3.7...</div><div><br></div><div>The logic I'm using is that trunk should stay *green*, and should work out-of-the box for users. Until the OpenMP runtimes work out of the box, we shouldn't flip the default.</div></div></div>