[Openmp-dev] PPC64 patch from Intel's fourth cmake patch

Thu Aug 7 11:27:41 PDT 2014

Jack,

From http://www.cmake.org/Wiki/CMake_Useful_Variables
“
CMAKE_C_COMPILER
the compiler used for C files. Normally it is detected and set during the CMake run, but you can override it at configuration time. Note! It can not be changed after the first cmake or ccmake run. Although the gui allows to enter an alternative, it will be ignored in the next 'configure' run. Use for example:
CC=gcc-3.3 CXX=g++-3.3 cmake
to set the compiler. (You can also set CMAKE_C_COMPILER_INIT, before any PROJECT() or ENABLE_LANGUAGE() command.) Any other way (like writing make CC=gcc-3.3 CXX=g++-3.3) will not work. When using distcc or similar tools, you need to write:
CC="distcc gcc-3.3" CXX="distcc g++-3.3" cmake
However, this will empty all your CMAKE_..._FLAGS_... above.
”

Note that “It can not be changed after the first cmake or ccmake run” .  This means it cannot be specified again *even if it is the same compiler name*.

You should get this undesired message if you specify the compilers on a subsequent cmake run:
-- Configuring done
You have changed variables that require your cache to be deleted.
Configure will be re-run and you may have to reset some variables.
The following variables have changed:
CMAKE_C_COMPILER= clang
CMAKE_CXX_COMPILER= clang++

Your error message suggests that cmake set the compiler path to /Users/howarth/llvm-svn/openmp-3.6.0/runtime/build/clang which I’m guessing does not exist!

So you should be able to do this without any rm –rf * command:
cd openmp-3.6.0/runtime
mkdir build
cd build
cmake -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -Dos=mac -Darch=32 ..
make -j 1
cmake -Darch=32e ..
make -j 1
make fat

-- Johnny

From: openmp-dev-bounces at cs.uiuc.edu [mailto:openmp-dev-bounces at cs.uiuc.edu] On Behalf Of Jack Howarth
Sent: Thursday, August 7, 2014 11:40 AM
To: Cownie, James H
Cc: Michael Wong; Carlo Bertolli; openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: Re: [Openmp-dev] PPC64 patch from Intel's fourth cmake patch

Jamie,
     It seems that the latest commits for ppc64 seem to have regressed the fat build on darwin. After the commit of

http://lists.cs.uiuc.edu/pipermail/openmp-commits/2014-August/000030.html

on x86_64-apple-darwin11, I could still build the fat libiomp5 using…

cd openmp-3.6.0/runtime
mkdir build
cd build
cmake -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -Dos=mac -Darch=32 ..
make -j 1
cmake -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -Dos=mac -Darch=32e ..
make -j 1
make fat

However after the ppc64 changes went it, the build regressed such that I have to use 'rm -fr *' before the second call to cmake for -Darch=32e
otherwise, I get…

[  2%] Generating iomp_lib.h
[  5%] Generating ../exports/mac_32e/include_compat/iomp_lib.h
[  5%] Built target inc
[  8%] Generating kmp_i18n_id.inc
[ 11%] Generating kmp_i18n_default.inc
[ 13%] Generating omp.h
[ 13%] Built target needed-headers
Scanning dependencies of target iomp5
[ 16%] Building C object CMakeFiles/iomp5.dir/src/kmp_ftn_cdecl.c.o
/bin/sh: /Users/howarth/llvm-svn/openmp-3.6.0/runtime/build/clang: No such file or directory
make[2]: *** [CMakeFiles/iomp5.dir/src/kmp_ftn_cdecl.c.o] Error 127
make[1]: *** [CMakeFiles/iomp5.dir/all] Error 2
make: *** [all] Error 2

on the second 'make' for -Darch=32e

         Jack

On Thu, Aug 7, 2014 at 6:24 AM, Cownie, James H <james.h.cownie at intel.com<mailto:james.h.cownie at intel.com>> wrote:
Committed at Rev 215093.
I’ll now also commit to the 3.5 branch.

-- Jim

James Cownie <james.h.cownie at intel.com<mailto:james.h.cownie at intel.com>>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438<tel:%2B44%20117%209071438>

From: Carlo Bertolli [mailto:cbertol at us.ibm.com<mailto:cbertol at us.ibm.com>]
Sent: Wednesday, August 06, 2014 4:28 PM
To: Cownie, James H
Cc: C. Bergström; Michael Wong; openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:openmp-dev at dcs-maillist2.engr.illinois.edu>; Hal Finkel
Subject: RE: [Openmp-dev] PPC64 patch from Intel's fourth cmake patch

Hi James,

Attached a second version of my patch addressing C. Bergström concerns (again, thanks for this).

(See attached file: ppc64-second-patch-from-fourth-intel-cmake)

There were issues that remained unsolved - here are my answers:

1. I removed the use of uname from everywhere. The user has to specify the value of the variable "arch" on the command line (e.g. ppc64), otherwise 32e is assumed.

2. The old cmake has a very complex way of setting the OpenMP version (3 variables are used). I removed all my attempts to make them homogeneous, and left it as it was before I worked on it.

3. I left the implementation of kmp_invoke_microtask as Hal Finkel did it because there is no time to implement a better version - incidentally, this will work for the foreseeable future.

Please, let me know of any comments and I'll quickly fix the thing and send a revised patch.
In the meantime, thanks for holding this while I was fixing my patch.

Cheers

-- Carlo

[Inactive hide details for "Cownie, James H" ---08/06/2014 05:23:22 AM---Carlo, If you send a new patch soon, I'll commit it *be]"Cownie, James H" ---08/06/2014 05:23:22 AM---Carlo, If you send a new patch soon, I'll commit it *before* I push to the 3.5 branch.

From: "Cownie, James H" <james.h.cownie at intel.com<mailto:james.h.cownie at intel.com>>
To: Carlo Bertolli/Watson/IBM at IBMUS, C. Bergström <cbergstrom at pathscale.com<mailto:cbergstrom at pathscale.com>>
Cc: "openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:openmp-dev at dcs-maillist2.engr.illinois.edu>" <openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:openmp-dev at dcs-maillist2.engr.illinois.edu>>, Michael Wong <michaelw at ca.ibm.com<mailto:michaelw at ca.ibm.com>>
Date: 08/06/2014 05:23 AM
Subject: RE: [Openmp-dev] PPC64 patch from Intel's fourth cmake patch

________________________________

Carlo,

If you send a new patch soon, I’ll commit it *before* I push to the 3.5 branch.

-- Jim

James Cownie <james.h.cownie at intel.com<mailto:james.h.cownie at intel.com>>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438<tel:%2B44%20117%209071438>

From: openmp-dev-bounces at cs.uiuc.edu<mailto:openmp-dev-bounces at cs.uiuc.edu> [mailto:openmp-dev-bounces at cs.uiuc.edu] On Behalf Of Carlo Bertolli
Sent: Tuesday, August 05, 2014 11:52 PM
To: C. Bergström
Cc: openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:openmp-dev at dcs-maillist2.engr.illinois.edu>; Michael Wong
Subject: Re: [Openmp-dev] PPC64 patch from Intel's fourth cmake patch

Hi C. Bergstrom,

Thanks *very much* for the review. Below, my detailed comments on this, interspersed with your comments:

"C. Bergström" <cbergstrom at pathscale.com<mailto:cbergstrom at pathscale.com>> wrote on 08/05/2014 05:11:10 PM:

> From: "C. Bergström" <cbergstrom at pathscale.com<mailto:cbergstrom at pathscale.com>>
> To: Carlo Bertolli/Watson/IBM at IBMUS
> Cc: openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:openmp-dev at dcs-maillist2.engr.illinois.edu>, Michael Wong
> <michaelw at ca.ibm.com<mailto:michaelw at ca.ibm.com>>
> Date: 08/05/2014 05:13 PM
> Subject: Re: [Openmp-dev] PPC64 patch from Intel's fourth cmake patch
>
> On 08/ 6/14 01:26 AM, Carlo Bertolli wrote:
> >
> > Hi all,
> >
> > I managed to create a git patch on top of latest Intel's cmake patch
> > from Jonathan Peyton.
> >
> > /(See attached file: ppc64-patch-from-fourth-intel-cmake)/
> >
> > This patch adds the ppc64 architecture and enables the current
> > Makefile system, the old cmake system, and the new cmake system on
> > that architecture.
> > It completely replaces the previous patch I sent on the openmp-commits
> > list.
> >
> >
> > Please let me know of any comments.
> >
> Hi Carlo!
>
> Thanks a lot for sending this
>
> Here's my off the cuff review
> ---------
> using uname to detect the host/target was rejected before. Please use
> the "cmake" way of detecting things.
> +EXECUTE_PROCESS( COMMAND uname -m COMMAND tr -d '\n' OUTPUT_VARIABLE
> DETECT_ARCH )
> +EXECUTE_PROCESS( COMMAND uname -m COMMAND tr -d '\n' OUTPUT_VARIABLE ARCH )
>

OK. I did some investigation on this and have done it again just now but was not able to find a way of doing this natively in cmake.
Actually, I found comments about how cmake does not support this natively (may be an old cmake version).
Can you suggest how to do this? (a link to a web page with an example is fine too).

>
> Please don't use !not when it can be avoided - It can be slightly
> confusing at a glance
> # Is this a work-around to something? Why not just test for ppc64 and
> handle that condition?
> if(NOT "${DETECT_ARCH}" STREQUAL "ppc64")

I agree this is just a stupid way of coding things - I will fix it.

>
> why change the minimum cmake version?
> +cmake_minimum_required(VERSION 3.0)

I got an annoying warning from cmake when running on one of the machines here - I added it to get rid to the warning, but I will delete this.

>
> --------
> We should find a correct solution instead of "ugly work-around" - I hope
> Intel can chime in here
> +#ugly workaround because these variables are not really set
> // For PathScale on BGQ - we are only able to support OMP2.5 for now.
> Advertising support all the way to 4.x would possibly lead to weird
> things. This should be done with care

This is a quick fixup that I made to the old cmake to have it work again on the machine I was using. I can get rid of those and ask who runs cmake to set them up. Would that be fine by you?

> --------
> I don't know if this is limited to linux only, but it isn't a correct
> assumption for Solaris and possibly FreeBSD
> -  set(CMAKE_SHARED_LINKER_FLAGS
> "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports_so.txt")
> +  set(CMAKE_SHARED_LINKER_FLAGS
> "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/exports_so.txt -ldl")
>

I think this only triggered an error on ppc64 linux. x86_64 linux did not raise any issue. What is your suggestion? Should I guard it as "if linux && ppc64"?

> --------
> I think this sneaked in
> -#define KMP_VERSION_BUILD    00000000
> +//#define KMP_VERSION_BUILD    00000000
>

Yep, it did!

> --------
> I don't have the time to review makefile.mk/perl<http://makefile.mk/perl> module - someone else
> should do that..
> # Personally, I think it should be removed

runtime/src/makefile.mk<http://makefile.mk> is used by the current Makefile (not sure about cmakes). In general, I am happier if Makefile stays in its place.

> ----------
>
> // This looks really weird and I'd need more details before accepting it
> // I also think we may want to limit this damage to very specifically
> "clang" instead of just PPC64
> // Please provide more details
> +__kmp_invoke_microtask
>

About this: I am not aware of any compiler that will target PPC64 using IOMP, except CLANG (I am happy to be told I am wrong). CLANG does currently gather all arguments to the microtask into a struct of pointers. That is the reason for my comment. There is ongoing discussion at Intel, I think, about why this is/is not bad.

From PPC64 perspective, we are happy the way it is. This implementation is coming from Hal Finkel (I am cc-ing him) that actually implemented that function. I had some discussion with my colleagues here and, if eventually this implementation gets in the way of getting better performance, we shall be able to provide an assembly version of this.
For this patch, we feel comfortable as it is.

>
> How do you plan to test this?
> Does it pass the testsuite which has been made available?
> I believe ANL did some work on this (Jeff Hammond) - is any of his work
> used in this?

Yes, it passes the testsuite with similar results as with gcc on x86_64. If you need them, I can provide detailed information.
Once we have clarified the remaining details, I will provide a new patch.

Thanks again for your detailed review and apologies about my inexperience in llvm-related projects.

Cheers

-- Carlo

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

_______________________________________________
Openmp-dev mailing list
Openmp-dev at dcs-maillist2.engr.illinois.edu<mailto:Openmp-dev at dcs-maillist2.engr.illinois.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/openmp-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20140807/beaf61ee/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.gif
Type: image/gif
Size: 105 bytes
Desc: image001.gif
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20140807/beaf61ee/attachment.gif>