This "discussion" was posted to -commits and not here - (which is not
cool for something this size)
+Jonathan
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
First, I want the code to reach as many people as possible including pathcc users. So on my own time, I have downloaded the Pathscale compiler from: http://www.pathscale.com/ekopath-compiler-suite which I did not know was open sourced, and tried to build libomp with pathcc on Linux, x86_64 (Yes, still Intel world in terms of architecture, but your original complaint suggested it was with the compiler) with Chandler's latest flag checks and succeeded with the log of the entire process attached.
There's numerous warnings about deprecated register storage (-Wdeprecated-register), and with some further investigation this is caused by the -std=c++11 flag and as I recall, Chris, you were the driving force behind getting that changed. I don't see either -Wno-deprecated-register or -Wdeprecated-register flag available for the open sourced pathcc so I don't see how to fix that warning even though pathcc's warning message suggests it exists. It's certainly no showstopper.
I’ll reiterate once more. I feel the changes should stay.
-- Johnny
pathcc.log (38.2 KB)
#1 The patch fixed the problem. If you feel it's a good fix then push
it. I wasn't clear on the exact behavior.
#1 The patch fixed the problem. If you feel it's a good fix then push it. I wasn't clear on the exact behavior.
If you're referring to the patch from: http://lists.cs.uiuc.edu/pipermail/openmp-dev/2015-July/000791.html
Then something is amiss because that patch is solely for debugging purposes. Here is that patch in text form:
diff --git a/runtime/cmake/LibompCheckLinkerFlag.cmake b/runtime/cmake/LibompCheckLinkerFlag.cmake
index 1c4a085..ec49a8b 100644
--- a/runtime/cmake/LibompCheckLinkerFlag.cmake
+++ b/runtime/cmake/LibompCheckLinkerFlag.cmake
@@ -23,7 +23,8 @@ function(libomp_check_linker_flag flag boolean)
set(CMAKE_SHARED_LINKER_FLAGS \"${flag}\")
add_library(foo SHARED src_to_link.c)")
set(failed_regexes "[Ee]rror;[Uu]nknown;[Ss]kipping;LINK : warning")
- set(base_dir ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/link_flag_check)
+ #set(base_dir ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/link_flag_check)
+ set(base_dir ${CMAKE_BINARY_DIR}/check_${boolean})
file(MAKE_DIRECTORY ${base_dir})
file(MAKE_DIRECTORY ${base_dir}/build)
file(WRITE ${base_dir}/src_to_link.c "${library_source}")
This patch only changes where the check takes place. It leaves the directories and files behind for the user to debug and see why a linker flag check is failing or passing when it shouldn't be.
This patch does not fix anything.
From the log I posted before (pathcc.log):
-- Performing Test LIBOMP_HAVE_STATIC_LIBGCC_FLAG
-- Performing Test LIBOMP_HAVE_STATIC_LIBGCC_FLAG - Failed
This suggests that the pathcc free binary (5.0.5) works as you've suggested (pathcc doesn't support the flag), and it doesn't include it. This was run without the debugging patch above.
If other compilers are compatible with gcc link flags - I doubt they would see a problem.
This isn't the point of a CMake configure/build system even if the main compiler support is gcc/clang. The CMake configure/build system is set up so compilers only include flags they support. That's the purpose of the checks and conforms to the other llvm projects. You've mentioned the -static-libgcc flag and nothing else and I'm trying to see why pathcc would include it, but can't reproduce the problem and neither can you as far as I can tell.
Long term I'd be fine and probably prefer you to just detect the PathScale compiler and not set anything.
This seems completely non-user-friendly to the rest of the pathcc users. I'd rather just fix the actual problem if one exists which I'm still not convinced there is.
If *you* need the CMake to be very particular for compiler flags for your particular setup, then you can create a PathScaleKillFlags.cmake file which clears all the LIBOMP_HAVE_XYZ_FLAGS, include it right after the include(config-ix) in the main CMakeLists.txt, then they won't be included and it is somewhat modular. Or, you can come up with a better solution for your problem.
To further prove that this new system should stay in I've now tried to build using pathcc with the old CMake system before the refactor and I get a warning:
CMake Warning at cmake/HelperFunctions.cmake:31 (message):
Could not find cmake/PathScale/CFlags.cmake: will only use default flags
Call Stack (most recent call first):
CMakeLists.txt:478 (warning_say)
CMake Warning at cmake/HelperFunctions.cmake:31 (message):
Could not find cmake//AsmFlags.cmake: will only use default flags
Call Stack (most recent call first):
CMakeLists.txt:502 (warning_say)
And then the build ***fails*** on the first source file with:
Scanning dependencies of target omp
make[2]: warning: Clock skew detected. Your build may be incomplete.
make[2]: Warning: File `src/CMakeFiles/omp.dir/flags.make' has modification time 67 s in the future
[ 20%] Building C object src/CMakeFiles/omp.dir/kmp_ftn_cdecl.c.o
In file included from /nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp_ftn_cdecl.c:16:
In file included from /nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp.h:88:
/nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp_lock.h:385:5: error: must use 'struct' tag to refer to type 'kmp_base_queuing_lock'
kmp_base_queuing_lock qlk;
^
struct
In file included from /nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp_ftn_cdecl.c:16:
/nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp.h:1928:10: error: unknown type name 'bool'
bool in:1;
^
/nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp.h:1929:10: error: unknown type name 'bool'
bool out:1;
^
/nfs/fx/proj/openmp/users/jlpeyton/openmp_llvm_commit/itt/libomp_llvm/runtime/src/kmp.h:2694:8: error: must use 'struct' tag to refer to type 'kmp_adaptive_backoff_params_t'
extern kmp_adaptive_backoff_params_t __kmp_adaptive_backoff_params;
^
struct
Since I've been *able* to build with pathcc using the new CMake system but *not able* to build with the old CMake system, I'd say these CMake refactor changes are for the best for the general pathcc user.
-- Johnny
My observation is that it's not consistently working and that's why
I'm even more upset and nervous about this change.
Further, "it works for me" is a very bad reason to go screw up a
stable branch and to continue to advocate that it so strongly.
The reason the release branch (before the merge commits) "works for
me" is because we have a wrapper cmake around the the project which
sets things correctly.
Just some observations:
1) The latest and greatest pathcc (6.0.449) builds fine. Not really sure how a compiler from March is "really old" but ok. No problem detecting that -static-libgcc flag is not supported... Still the same warnings about the deprecated register storage.
2) Your build system is completely unique. I have NO idea what's going based on what you've posted. So "it doesn't work for my site-specific custom wrapper scripts" isn't a good reason to block a patch that vastly improves the CMake build system.
This -> checking whether we are using the GNU C++ compiler... yes checking whether /opt/ekopath/bin/pathCC accepts -g... yes checking whether g++ accepts -static-libstdc++ -static-libgcc... no checking for gnatbind... no
is *autoconf* output (or your own personal shell configure script?), not CMake output. CMake would recognize the pathcc compiler as a Pathscale compiler.
3) Are you really trying to advocate that your site-specific, custom CMake (and possibly autoconf?) wrapper configure/build script(s) should magically work with what's in the repository? *YOU* have to keep that up to date and have it working with what is in the source. It is not my responsibility to ensure that everyone's custom cmake wrapper is compatible with the repo in the CMake.
4) I'm happy to test things out.
5) On this website: http://www.pathscale.com/ekopath-compiler-suite there are no instructions regarding how to download and change the URL date.
6) Nothing you've said has convinced me that the refactored CMake build system shouldn't be in the 3.7 release branch. In fact, I'm now convinced that you have some odd setup which I can't possibly help fix/replicate, but is completely unique to your site. I don't feel that a custom CMake wrapper based build system should warrant the blocking of a patch.
*****Either use what's in the LLVM repo, or fix your custom scripts and submit a patch.*****
Unless it's LLVM policy to cater to every custom build wrapper script, or you show me a bug from *using the build system in the LLVM repo with no wrapper scripts*, then this patch should stay.
-- Johnny
C Bergström,
It would be helpful if you were more explicit about how exactly
you are building openmp 3.7.0svn. Is this simply a stand-alone build
of openmp 3.7.0svn against the pathscale compilers or are you doing
the build in-tree as part of a complete
llvm/cfe/compiler-rt/libc++/openmp build? Also, are the failures in
the invocation of cmake or the later invocation of make. If the later,
are you using parallel builds or serial builds in make? Daily
three-stage bootstraps of llvm/cfe/compiler-rt/polly/libc++/polly on
x86_64-apple-darwin from 3.7.0 branch show no instabilities in -j8
parallel makes here. Lastly, what exact options are you passing to
cmake?
Jack
My observation is that it's not consistently working and that's why
I'm even more upset and nervous about this change.Further, "it works for me" is a very bad reason to go screw up a
stable branch and to continue to advocate that it so strongly.
I've tried to reproduce the problem described here by hacking up a
local clang that doesn't support the -static-libgcc flag. I verified
that the cmake correctly identifies that the flag is not supported,
that it's not used during the build, and doesn't show up in the build
file (it would show in build/runtime/src/CMakeFiles/omp.dir/link.txt
if supported). I also didn't see anything wrong in the cmake file.
I'm sympathetic to your build issue, but I do think the onus is on you
to show that something is broken here in a way that the community can
reproduce or at least understand.
The reason the release branch (before the merge commits) "works for
me" is because we have a wrapper cmake around the the project which
sets things correctly.
-------------
Here's what I seeFrom cmake # Everything would appear to look good, right? (Also know
that it says g++, but in fact it's really pathcc.. I don't quite
understand that)
-------------------
checking whether we are using the GNU C++ compiler... yes
checking whether /opt/ekopath/bin/pathCC accepts -g... yes
checking whether g++ accepts -static-libstdc++ -static-libgcc... no
checking for gnatbind... no
The above looks like output from a configure script, not CMake.
Where's that coming from?
- Hans
My bad on including the autocrap output. We have cmake also building
binutils which does some autoconf stuff and its output ends up in our
cmake.log - (which detection is working correctly)
From: "C Bergström" <cbergstrom@pathscale.com>
To: "Hans Wennborg" <hans@chromium.org>
Cc: openmp-commits@dcs-maillist2.engr.illinois.edu, openmp-dev@dcs-maillist2.engr.illinois.edu
Sent: Tuesday, July 21, 2015 3:33:51 PM
Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake build systemMy bad on including the autocrap output. We have cmake also building
binutils which does some autoconf stuff and its output ends up in our
cmake.log - (which detection is working correctly)
--------
Here's hopefully correct information
Summary - link.txt contains -static-libgccopenmp-llvm/build-x86_32/src/CMakeFiles/omp.dir/link.txt
---------
/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/openmp-llvm/build-x86_32/src/CMakeFiles/omp.dir/link.txt:/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/Xcompiler/bin/pathcc
-fPIC -g -O0 -Wl,--warn-shared-textrel -Wl,--as-needed
-Wl,--version-script=/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/src/enzo-suite/ekopath/openmp-llvm/runtime/src/exports_so.txt
-static-libgcc -Wl,-z,noexecstack -Wl,-fini=__kmp_internal_end_fini
-L/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/Xcompiler/lib/6.0.26/x8664/32
-msse2 -shared -Wl,-soname,libomp.so -o libomp.so
CMakeFiles/omp.dir/kmp_alloc.c.o CMakeFiles/omp.dir/kmp_atomic.c.o
CMakeFiles/omp.dir/kmp_csupport.c.o CMakeFiles/omp.dir/kmp_debug.c.o
CMakeFiles/omp.dir/kmp_itt.c.o CMakeFiles/omp.dir/kmp_environment.c.o
CMakeFiles/omp.dir/kmp_error.c.o CMakeFiles/omp.dir/kmp_global.c.o
CMakeFiles/omp.dir/kmp_i18n.c.o CMakeFiles/omp.dir/kmp_io.c.o
CMakeFiles/omp.dir/kmp_runtime.c.o
CMakeFiles/omp.dir/kmp_settings.c.o
CMakeFiles/omp.dir/kmp_str.c.o CMakeFiles/omp.dir/kmp_tasking.c.o
CMakeFiles/omp.dir/kmp_taskq.c.o
CMakeFiles/omp.dir/kmp_threadprivate.c.o
CMakeFiles/omp.dir/kmp_utility.c.o
CMakeFiles/omp.dir/z_Linux_util.c.o
CMakeFiles/omp.dir/kmp_gsupport.c.o
CMakeFiles/omp.dir/thirdparty/ittnotify/ittnotify_static.c.o
CMakeFiles/omp.dir/kmp_ftn_cdecl.c.o
CMakeFiles/omp.dir/kmp_ftn_extra.c.o
CMakeFiles/omp.dir/kmp_version.c.o
CMakeFiles/omp.dir/kmp_barrier.cpp.o
CMakeFiles/omp.dir/kmp_wait_release.cpp.o
CMakeFiles/omp.dir/kmp_affinity.cpp.o
CMakeFiles/omp.dir/kmp_dispatch.cpp.o
CMakeFiles/omp.dir/kmp_lock.cpp.o CMakeFiles/omp.dir/kmp_sched.cpp.o
CMakeFiles/omp.dir/kmp_taskdeps.cpp.o
CMakeFiles/omp.dir/kmp_cancel.cpp.o
CMakeFiles/omp.dir/z_Linux_asm.s.o
-lpthread -ldl
/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/openmp-llvm/build-x86_64/src/CMakeFiles/omp.dir/link.txt:/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/Xcompiler/bin/pathcc
-fPIC -g -O0 -Wl,--warn-shared-textrel -Wl,--as-needed
-Wl,--version-script=/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/src/enzo-suite/ekopath/openmp-llvm/runtime/src/exports_so.txt
-static-libgcc -Wl,-z,noexecstack -Wl,-fini=__kmp_internal_end_fini
-L/bamboo-agent-home/xml-data/build-dir/EKOPATH-EKOPATH6X8632-DEBUG/build/Xcompiler/lib/6.0.26/x8664/64
-msse2 -shared -Wl,-soname,libomp.so -o libomp.so
CMakeFiles/omp.dir/kmp_alloc.c.o CMakeFiles/omp.dir/kmp_atomic.c.o
CMakeFiles/omp.dir/kmp_csupport.c.o CMakeFiles/omp.dir/kmp_debug.c.o
CMakeFiles/omp.dir/kmp_itt.c.o CMakeFiles/omp.dir/kmp_environment.c.o
CMakeFiles/omp.dir/kmp_error.c.o CMakeFiles/omp.dir/kmp_global.c.o
CMakeFiles/omp.dir/kmp_i18n.c.o CMakeFiles/omp.dir/kmp_io.c.o
CMakeFiles/omp.dir/kmp_runtime.c.o
CMakeFiles/omp.dir/kmp_settings.c.o
CMakeFiles/omp.dir/kmp_str.c.o CMakeFiles/omp.dir/kmp_tasking.c.o
CMakeFiles/omp.dir/kmp_taskq.c.o
CMakeFiles/omp.dir/kmp_threadprivate.c.o
CMakeFiles/omp.dir/kmp_utility.c.o
CMakeFiles/omp.dir/z_Linux_util.c.o
CMakeFiles/omp.dir/kmp_gsupport.c.o
CMakeFiles/omp.dir/thirdparty/ittnotify/ittnotify_static.c.o
CMakeFiles/omp.dir/kmp_ftn_cdecl.c.o
CMakeFiles/omp.dir/kmp_ftn_extra.c.o
CMakeFiles/omp.dir/kmp_version.c.o
CMakeFiles/omp.dir/kmp_barrier.cpp.o
CMakeFiles/omp.dir/kmp_wait_release.cpp.o
CMakeFiles/omp.dir/kmp_affinity.cpp.o
CMakeFiles/omp.dir/kmp_dispatch.cpp.o
CMakeFiles/omp.dir/kmp_lock.cpp.o CMakeFiles/omp.dir/kmp_sched.cpp.o
CMakeFiles/omp.dir/kmp_taskdeps.cpp.o
CMakeFiles/omp.dir/kmp_cancel.cpp.o
CMakeFiles/omp.dir/z_Linux_asm.s.o
-lpthread -ldl
--------
For clarification - by "unstable" I mean this does work on some
hosts,
but not on others. I've tried to triage it down to specific versions
of cmake, but so far I can't find why it fails only on certain hosts.
(Yes this drives me crazy too)We will probably open source some of these cmake wrappers in the near
future (1-2 months), but what they do is basically thisTop level cmake which just includes sub cmake directories. This
allows
us to take a "just built" compiler and then build the necessary
runtime libraries. (we loop all specified targets - in the above
example that's 32 and 64bit x86)So it's not a real "in-tree" llvm build, but it is "in-tree" in the
cmake sense. (I guess it would fall more on the side of standalone
build)-j doesn't impact this. It's a configure time problem.
I really do appreciate the time others are taking on this. This isn't
an imaginary problem. Again - "works for me" isn't a good criteria to
merge stuff to a stable branch.
I don't believe that your problem is imaginary, but we also need to be able to reproduce it, or have a patch fixing it with some explanation of what's going on. Regardless, it is really important for the LLVM community to move forward with the new build system. We certainly have a release-candidate process, and the purpose of this process is to isolate and correct bugs. In that sense, the system is working as intended. The new CMake build system is in line with LLVM's general best practices, and the community has demanded that as a prerequisite for moving forward with integration with Clang. As a result, we really do need to get user feedback on this build system as opposed to any other.
-Hal
I don't believe that your problem is imaginary, but we also need to be able to reproduce it, or have a patch fixing it with some explanation of what's going on. Regardless, it is really important for the LLVM community to move forward with the new build system. We certainly have a release-candidate process, and the purpose of this process is to isolate and correct bugs. In that sense, the system is working as intended. The new CMake build system is in line with LLVM's general best practices, and the community has demanded that as a prerequisite for moving forward with integration with Clang. As a result, we really do need to get user feedback on this build system as opposed to any other.
-Hal
Ok - so standalone user hitting a critical blocking issue isn't
important enough feedback to consider this a blocker.
I didn't say revert the whole thing - I said leave it out of the
"STABLE" 3.7 branch.
From: "C Bergström" <cbergstrom@pathscale.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: openmp-commits@dcs-maillist2.engr.illinois.edu, openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
<hans@chromium.org>
Sent: Tuesday, July 21, 2015 5:41:55 PM
Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake build system> I don't believe that your problem is imaginary, but we also need to
> be able to reproduce it, or have a patch fixing it with some
> explanation of what's going on. Regardless, it is really important
> for the LLVM community to move forward with the new build system.
> We certainly have a release-candidate process, and the purpose of
> this process is to isolate and correct bugs. In that sense, the
> system is working as intended. The new CMake build system is in
> line with LLVM's general best practices, and the community has
> demanded that as a prerequisite for moving forward with
> integration with Clang. As a result, we really do need to get user
> feedback on this build system as opposed to any other.
>
> -HalOk - so standalone user hitting a critical blocking issue isn't
important enough feedback to consider this a blocker.I didn't say revert the whole thing - I said leave it out of the
"STABLE" 3.7 branch.
The stable branch is not yet stable, it is stabilizing. It was, after all, forked from trunk not long ago. We're now going through the process of isolating and fixing bugs. This is how the system works, and your feedback is part of that process.
-----------
Everyone here is totally cool with this difficult to reproduce bug
that was introduced in the stable branch at the last minute.
(Actually
days after the tagged release)
We tagged the branch, we did not have a release candidate until much later.
If that's the way this project will work well then I guess my
feedback
and effort isn't worth shit. It would be far less effort for us to
fork than waste my time and yours.We don't hit this issue in any other project (libc++, compiler-rt or
others). So clearly the openmp cmake is doing something different
from
those.
--------
To stop wasting time on this - I see the following options and I let
Hans or someone else in a leadership position decide1) We fork off - going forward I will have nothing to do with this
project2) revert the offending changes in the 3.7 release branch (preferred)
3) introduce a fix asap (unlikely since the root problem hasn't been
identified)4) Introduce #if PathScale logic in the cmake files to avoid setting
any flags when our compiler it detected. (2nd preference) The patch
for this should be trivial and if things are designed correctly
should
take less time than the average email in this thread. I see there's
logic for Intel compiler and others already. I don't like the
spaghetti mess, but c'est la vie
Frankly, I don't understand your position. You ship a product based, in part, on LLVM software. So do I. I certainly have patches that I apply to the upstream sources to deal with build/environment customizations that, for several different reasons, are not appropriate for the upstream repository. Nevertheless, I don't view that as being in conflict with contributing to the community. You can clearly do whatever you need to in order to fix this problem locally. If the problem turns out to be a problem with the upstream build system, clearly we'll act to correct it. Right now, we don't have enough information to act in that regard.
We appreciate your feedback, and we're well aware that testing during the release cycle takes time and effort on everyone's part.
-Hal
>
> -HalOk - so standalone user hitting a critical blocking issue isn't
important enough feedback to consider this a blocker.I didn't say revert the whole thing - I said leave it out of the
"STABLE" 3.7 branch.The stable branch is not yet stable, it is stabilizing. It was, after all, forked from trunk not long ago. We're now going through the process of isolating and fixing bugs. This is how the system works, and your feedback is part of that process.
I thought the branch was tagged and "final". Maybe I'm mistaken on
this process since I'm not typically involved with the main clang/llvm
release process.
Until now - we have had no plans to maintain a fork. I've been solely
focused on getting all changes upstream and doing production releases
directly off the main repo. (Either stable branch or master)
In fact I'm surprised and discouraged to hear about patches which
aren't going upstream. So far there hasn't been any untenable
technical reason to not contribute it from our side. I'd be very
interested to know what patches you're maintaining separately and why.
From: "C Bergström" <cbergstrom@pathscale.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: openmp-commits@dcs-maillist2.engr.illinois.edu, openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
<hans@chromium.org>
Sent: Tuesday, July 21, 2015 6:31:58 PM
Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake build system>> >
>> > -Hal
>>
>> Ok - so standalone user hitting a critical blocking issue isn't
>> important enough feedback to consider this a blocker.
>>
>> I didn't say revert the whole thing - I said leave it out of the
>> "STABLE" 3.7 branch.
>
> The stable branch is not yet stable, it is stabilizing. It was,
> after all, forked from trunk not long ago. We're now going through
> the process of isolating and fixing bugs. This is how the system
> works, and your feedback is part of that process.I thought the branch was tagged and "final". Maybe I'm mistaken on
this process since I'm not typically involved with the main
clang/llvm
release process.
Ah, no. We've tagged the release branch, but we then draw release candidates from that branch. If you look in the box labeled at 'Upcoming Releases' on the front page of http://llvm.org/, you'll see the current target schedule:
14 July 2015: Branch for 3.7.0 release
14 July–21 July: Testing Phase I
22 July–29 July: Fix bugs from Testing Phase I
30 July–6 August: Testing Phase II
7 August–14 August: Fix bugs from Testing Phase II
21 August: Release LLVM 3.7.0.
As release candidates are made, and fixes are produced, they're merged into the release branch.
Until now - we have had no plans to maintain a fork. I've been solely
focused on getting all changes upstream and doing production releases
directly off the main repo. (Either stable branch or master)In fact I'm surprised and discouraged to hear about patches which
aren't going upstream. So far there hasn't been any untenable
technical reason to not contribute it from our side. I'd be very
interested to know what patches you're maintaining separately and
why.
These are patches to integrate Clang/LLVM with libraries and directory layouts that are only provided as part of the relevant bundle of software. All of bgclang is open source, however, and if there is something in there you feel should be upstream, and isn't, we should work on changing that. My TODO list in this area is not empty.
---------
If someone doesn't push the #if PathScale change - then I'll send a
patch in the next day or so. I expect it to be trivial. That's the
best middle ground I can hope for in the short term.
Sounds good; we have plenty of time to fix things before the actual release.
-Hal
#1 I don't think a "large refactor of cmake build system" should be
merged 2 days after the branch was cut. It really should only be for
bug fixes from what I can see
#2 I need to look more into how you're packaging stuff - the cmake
pieces I plan to open source may help with some of that. I remember
some of your patches in the past clobbered normal Power7 support and
hopefully that stuff is resolved. If it's just to do with directory
structures I certainly think I can help. (sode note - I owe you an
update for mira/vesta - coming soon)
From: "C Bergström" <cbergstrom@pathscale.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: openmp-commits@dcs-maillist2.engr.illinois.edu, openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
<hans@chromium.org>
Sent: Tuesday, July 21, 2015 7:16:46 PM
Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake build system>> From: "C Bergström" <cbergstrom@pathscale.com>
>> To: "Hal Finkel" <hfinkel@anl.gov>
>> Cc: openmp-commits@dcs-maillist2.engr.illinois.edu,
>> openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
>> <hans@chromium.org>
>> Sent: Tuesday, July 21, 2015 6:31:58 PM
>> Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake
>> build system
>>
>> >> >
>> >> > -Hal
>> >>
>> >> Ok - so standalone user hitting a critical blocking issue isn't
>> >> important enough feedback to consider this a blocker.
>> >>
>> >> I didn't say revert the whole thing - I said leave it out of
>> >> the
>> >> "STABLE" 3.7 branch.
>> >
>> > The stable branch is not yet stable, it is stabilizing. It was,
>> > after all, forked from trunk not long ago. We're now going
>> > through
>> > the process of isolating and fixing bugs. This is how the system
>> > works, and your feedback is part of that process.
>>
>> I thought the branch was tagged and "final". Maybe I'm mistaken on
>> this process since I'm not typically involved with the main
>> clang/llvm
>> release process.
>
> Ah, no. We've tagged the release branch, but we then draw release
> candidates from that branch. If you look in the box labeled at
> 'Upcoming Releases' on the front page of http://llvm.org/, you'll
> see the current target schedule:
>
> 14 July 2015: Branch for 3.7.0 release
> 14 July–21 July: Testing Phase I
> 22 July–29 July: Fix bugs from Testing Phase I
> 30 July–6 August: Testing Phase II
> 7 August–14 August: Fix bugs from Testing Phase II
> 21 August: Release LLVM 3.7.0.
>
> As release candidates are made, and fixes are produced, they're
> merged into the release branch.
>
>>
>> Until now - we have had no plans to maintain a fork. I've been
>> solely
>> focused on getting all changes upstream and doing production
>> releases
>> directly off the main repo. (Either stable branch or master)
>>
>> In fact I'm surprised and discouraged to hear about patches which
>> aren't going upstream. So far there hasn't been any untenable
>> technical reason to not contribute it from our side. I'd be very
>> interested to know what patches you're maintaining separately and
>> why.
>
> These are patches to integrate Clang/LLVM with libraries and
> directory layouts that are only provided as part of the relevant
> bundle of software. All of bgclang is open source, however, and if
> there is something in there you feel should be upstream, and
> isn't, we should work on changing that. My TODO list in this area
> is not empty.
>
>> ---------
>> If someone doesn't push the #if PathScale change - then I'll send
>> a
>> patch in the next day or so. I expect it to be trivial. That's the
>> best middle ground I can hope for in the short term.
>
> Sounds good; we have plenty of time to fix things before the actual
> release.#1 I don't think a "large refactor of cmake build system" should be
merged 2 days after the branch was cut. It really should only be for
bug fixes from what I can see
We need to start filtering at some point, but from the standpoint of moving the whole community forward, having this in 3.7 is the right decision. Now that Clang has completed its OpenMP 3.1 implementation, users are going to try building this OpenMP runtime library for use with Clang/LLVM. We want them to gain experience, and give us feedback on, the this build system, not the old one that needed replacement. Also, we knew this was coming for a long time, so it was a surprise to no one following the list. This is obviously a judgment call, but merging this a couple of days after the branch seems worth it. That still gives us over a month to find/fix bugs, and only a couple of days less than we have to do the same for the whole rest of the project.
#2 I need to look more into how you're packaging stuff - the cmake
pieces I plan to open source may help with some of that. I remember
some of your patches in the past clobbered normal Power7 support and
hopefully that stuff is resolved. If it's just to do with directory
structures I certainly think I can help. (sode note - I owe you an
update for mira/vesta - coming soon)
Sure, sounds good. My patchsets should now be clean re: clobbering other PowerPC things. Let's discuss this off-list.
-Hal
From: "C Bergström" <cbergstrom@pathscale.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: openmp-commits@dcs-maillist2.engr.illinois.edu, openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
<hans@chromium.org>
Sent: Tuesday, July 21, 2015 7:16:46 PM
Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake build system>> From: "C Bergström" <cbergstrom@pathscale.com>
>> To: "Hal Finkel" <hfinkel@anl.gov>
>> Cc: openmp-commits@dcs-maillist2.engr.illinois.edu,
>> openmp-dev@dcs-maillist2.engr.illinois.edu, "Hans Wennborg"
>> <hans@chromium.org>
>> Sent: Tuesday, July 21, 2015 6:31:58 PM
>> Subject: Re: [Openmp-commits] [Openmp-dev] Large Refactor of CMake
>> build system
>>
>> >> >
>> >> > -Hal
>> >>
>> >> Ok - so standalone user hitting a critical blocking issue isn't
>> >> important enough feedback to consider this a blocker.
>> >>
>> >> I didn't say revert the whole thing - I said leave it out of
>> >> the
>> >> "STABLE" 3.7 branch.
>> >
>> > The stable branch is not yet stable, it is stabilizing. It was,
>> > after all, forked from trunk not long ago. We're now going
>> > through
>> > the process of isolating and fixing bugs. This is how the system
>> > works, and your feedback is part of that process.
>>
>> I thought the branch was tagged and "final". Maybe I'm mistaken on
>> this process since I'm not typically involved with the main
>> clang/llvm
>> release process.
>
> Ah, no. We've tagged the release branch, but we then draw release
> candidates from that branch. If you look in the box labeled at
> 'Upcoming Releases' on the front page of http://llvm.org/, you'll
> see the current target schedule:
>
> 14 July 2015: Branch for 3.7.0 release
> 14 July–21 July: Testing Phase I
> 22 July–29 July: Fix bugs from Testing Phase I
> 30 July–6 August: Testing Phase II
> 7 August–14 August: Fix bugs from Testing Phase II
> 21 August: Release LLVM 3.7.0.
>
> As release candidates are made, and fixes are produced, they're
> merged into the release branch.
>
>>
>> Until now - we have had no plans to maintain a fork. I've been
>> solely
>> focused on getting all changes upstream and doing production
>> releases
>> directly off the main repo. (Either stable branch or master)
>>
>> In fact I'm surprised and discouraged to hear about patches which
>> aren't going upstream. So far there hasn't been any untenable
>> technical reason to not contribute it from our side. I'd be very
>> interested to know what patches you're maintaining separately and
>> why.
>
> These are patches to integrate Clang/LLVM with libraries and
> directory layouts that are only provided as part of the relevant
> bundle of software. All of bgclang is open source, however, and if
> there is something in there you feel should be upstream, and
> isn't, we should work on changing that. My TODO list in this area
> is not empty.
>
>> ---------
>> If someone doesn't push the #if PathScale change - then I'll send
>> a
>> patch in the next day or so. I expect it to be trivial. That's the
>> best middle ground I can hope for in the short term.
>
> Sounds good; we have plenty of time to fix things before the actual
> release.#1 I don't think a "large refactor of cmake build system" should be
merged 2 days after the branch was cut. It really should only be for
bug fixes from what I can seeWe need to start filtering at some point, but from the standpoint of moving the whole community forward, having this in 3.7 is the right decision. Now that Clang has completed its OpenMP 3.1 implementation, users are going to try building this OpenMP runtime library for use with Clang/LLVM. We want them to gain experience, and give us feedback on, the this build system, not the old one that needed replacement. Also, we knew this was coming for a long time, so it was a surprise to no one following the list. This is obviously a judgment call, but merging this a couple of days after the branch seems worth it. That still gives us over a month to find/fix bugs, and only a couple of days less than we have to do the same for the whole rest of the project.
IMNSHO
Users should not be building this library. This should be shipped with
the packaged compiler they are getting. Further, the packagers decide
what's the default and nothing prevented them from doing that in the
recent past.
From a developer perspective changing the default probably has no or
little impact on them. There's not a lot of people who are really
working on OpenMP runtimes. (Short list of companies and research
groups) That list gets even shorter when it comes to people using
*this* library.
In this case, "users" of the OpenMP build system means exactly those people who build it. Some of these are end users, some are packagers, some are its developers. It is these people from whom we need, and will, get feedback on the build system. From the developers we'll get feedback regardless, granted, but from the other groups it should be in the release.
-Hal