[PATCH] D11494: test-release.sh: Add -openmp option.

[Hans Wennborg]

hans added a comment.

In http://reviews.llvm.org/D11494#213401, @rengolin wrote:

> Hi Hans,
>
> Sorry, this fell out of my radar. I think it's an important addition, I just think it should be part of the main build and behave like the others. I have left some comments inline.
>
> Also, about the tar ball, are we going to start uploading both of them? Or is it maybe better to bundle the OpenMP tar inside the LLVM+Clang one? If we make this part of the main build, it may be easier to bundle all together.
>
> Either way is fine for me, but would be good if all target testers / code owners would agree on one for a reason. :)


I'm hoping that for the next release, the openmp builds and tests as any other LLVM module, by simply putting it under tools/ or projects. Currently it builds on the side and I'm not sure how to test it.

We could of course still work around that in the script and bake it into the regular LLVM+Clang tarball. However, I'm kind of hesitant to do so this late in the release process when none of us have really tested it.

My plan was to let those testers who wish try to build it, and then we'd upload the tarballs on the side, as a convenience for those who want to experiment with the openmp support.


================
Comment at: utils/release/test-release.sh:440
@@ +439,3 @@
+    cwd=`pwd`
+    set -x  # Echo on.
+
----------------
rengolin wrote:
> "set -x" considered harmful. :)
Removed.

================
Comment at: utils/release/test-release.sh:442
@@ +441,3 @@
+
+    rm -rf $BuildDir/openmp
+    rm -rf $BuildDir/openmp.install
----------------
rengolin wrote:
> This better stay in PhaseM/Release/... like the rest.
Done.

================
Comment at: utils/release/test-release.sh:446
@@ +445,3 @@
+    cd $BuildDir/openmp
+    clang=$BuildDir/Phase3/Release/llvmCore-$Release-$RC.install/usr/local/bin/clang
+    cmake -DCMAKE_C_COMPILER=${clang} -DCMAKE_CXX_COMPILER=${clang}++ \
----------------
rengolin wrote:
> Hasn't this moved out of /usr/local?
No, but we make sure the /usr/local prefix doesn't go in the tarball.

================
Comment at: utils/release/test-release.sh:454
@@ +453,3 @@
+
+    local Package=OpenMP-$Triple
+    mv $BuildDir/openmp.install/usr/local $BuildDir/$Package
----------------
rengolin wrote:
> Better not to override a global variable with the same name, it's confusing.
> 
> Call it OpenMPPackage or something.
Done.

================
Comment at: utils/release/test-release.sh:455
@@ +454,3 @@
+    local Package=OpenMP-$Triple
+    mv $BuildDir/openmp.install/usr/local $BuildDir/$Package
+    cd $BuildDir
----------------
rengolin wrote:
> I think this should be inside the main build process, so that we use the same standard openmp-3.7.0-install and then rename / tar like llvmCore-3.7.0-install, etc.
I've commented on that below. I'm not sure we're ready yet.


http://reviews.llvm.org/D11494