r238389 - [omp] Re-work Clang's handling of -fopenmp and undo r237769.

[Chandler Carruth]

On Thu, May 28, 2015 at 2:46 PM Andrey Bokhanko <andreybokhanko at gmail.com>
wrote:

> Chandler,
>
> Issue #1 -- not an issue at all. Personally, I *welcome* your fixes,
> and it's logical they are coming from you -- as apart of users you
> mentioned, there are zero documented users who *rely* on clang linking
> libgomp and ignoring all OMP pragmas.
>
> As for lack of code review, I trust your judgment and experience here
> (and still (!) ready to learn from you... -- remember?) and more than
> trust your driver fu.
>
> Issue #2 -- yes, I'm concerned. This seriously affects "the shortest
> path" user interface to OMP implementation and runtime library. I
> believe it warrants at least a discussion and some kind of consent in
> the community. Obviously, involving code owners of said OMP
> implementations would be reasonable.
>

I think that trunk should continue working for existing users during that
discussion.

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:

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

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.

I don't like (b) and would argue against it, but I also haven't seen anyone
who really does like (b).


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.


>
> > 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.
>
> That's surprising. We discussed this with Alexey and the very next day
> he responded with a fix that (as we hoped) resolves your concerns and
> gives an easy way to configure clang the way you and your users like.
> You replied to this fix with:
>
> > Yea, i think Daniel's patch was a closer start. Anyways, thanks for
> taking
> > a stab at this.
>
> (from http://reviews.llvm.org/D9875)
>
> I interpreted it as "I'm not 100% happy but that's OK". Is my
> interpretation wrong?
>

This only addressed one of the several complaints I raised in the review
thread. And the review thread itself was never updated.


>
> I *really* don't want us to become two boys shouting "it is libgomp!
> flip" and "no, it is libiomp! flip" to each other and flipping the
> switch ad infinitum. But I frankly fail to understand your logic...
>

I don't think that's going to happen -- I think the patch to flip the
default just went in too soon.

FTR, given the progress on the CMake and lit side of things, I wouldn't
expect this to even be risky for 3.7...

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150528/10a06756/attachment.html>