[Openmp-commits] [Openmp-dev] Large Refactor of CMake build system

[Peyton, Jonathan L]

I can’t provide a fix for a problem when I have no idea what it is.  I offered a way for Chris to provide more information about the problem by giving him a patch which helps debug the "broken" cmake logic.  The response I got back from Chris was the contradictory "With that patch it works as expected, but is that fully correct?".  These CMake patches vastly improve the libomp CMake build system making it more conducive with the overall build system of LLVM, and I strongly feel they should stay in the release branch.  The *only* reported problem has been from the Pathscale world which I can't test directly but have been more than willing to try to make work.

So honestly,  I'm unaware of any real problem.  I have no idea what I can possibly do until a real log, environment, etc. of the "problem" is shown.  I can't help from, "it's broken".

I know Chandler requested to put his recent change in the release branch as well so I'm CC'ing him.

Since I'm new and will obviously disagree with Chris about taking it out, it would be nice for others to chime in here.  I won't be offended if it's taken out.  I'll reiterate and am obviously a bit biased.  I feel it should stay in.

-- Johnny

-----Original Message-----
From: hwennborg at google.com [mailto:hwennborg at google.com] On Behalf Of Hans Wennborg
Sent: Monday, July 20, 2015 6:43 PM
To: C Bergström; Peyton, Jonathan L
Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: Re: [Openmp-dev] Large Refactor of CMake build system

+Jonathan

On Mon, Jul 20, 2015 at 12:02 PM, C Bergström <cbergstrom at pathscale.com> wrote:
> This "discussion" was posted to -commits and not here - (which is not 
> cool for something this size)
> --------------
>
> Original thread - sorry for messed up indenting Hi Jonathan,
>
> I think this is a reasonable request as we're still early in the 
> process and this can be considered a fix towards completing an 
> existing feature.
>
> I've merged this and the follow-up commit in r242335 and r242336, respectively.
>
> Thanks,
> Hans
>
> On Wed, Jul 15, 2015 at 2:29 PM, Peyton, Jonathan L <jonathan.l.peyton 
> at intel.com> wrote:
>> Hans,
>>
>> We were trying to get this into the 3.7.0 release so users could have this library be the default for the -fopenmp flag.  We missed the deadline by one day.  There were two commits that refactor the CMake code to be up to LLVM standards.  I would greatly appreciate if this (and the follow up commit r242301) were committed to the 3.7.0 openmp branch.  I know they are large, but they are key to getting the LLVM OpenMP Runtime library set as the default for clang.
>>
>> -- Johnny
>
> ----------
>
> This isn't really just about missing the deadline by 1 day. I stayed 
> out of the way for you to get the initial cmake changes in and counted 
> on doing post-commit testing and review (to clean-up whatever you 
> broke). (Chandler was already nitpicking you) This level of change 
> wouldn't fly in llvm/clang projects and I hope we can apply the same 
> general approach here. Basically it hasn't had any testing outside of 
> the Intel world - It would be nice if the Solaris, FreeBSD, Aarch, 
> Power8.. etc world had a chance to verify before it's pushed to a 
> stable branch. Ths sky won't fall over the -fopenmp flag.
>
> Please revert r242335 and r242336 in the branch and lets move forward 
> with testing and bug fixes.

I unfortunately hadn't seen the "Cmake changes" or "Broken cmake logic after recent changes" threads before doing the merge.

Jonathan, do you have a fix for this? Otherwise it should probably be reverted from the branch.

 - Hans