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

Renato Golin renato.golin at linaro.org
Tue Jul 28 04:44:03 PDT 2015


rengolin added a comment.

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. :)

cheers,
--renato


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

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

================
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}++ \
----------------
Hasn't this moved out of /usr/local?

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

Call it OpenMPPackage or something.

================
Comment at: utils/release/test-release.sh:455
@@ +454,3 @@
+    local Package=OpenMP-$Triple
+    mv $BuildDir/openmp.install/usr/local $BuildDir/$Package
+    cd $BuildDir
----------------
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.


http://reviews.llvm.org/D11494







More information about the Openmp-commits mailing list