[Openmp-dev] Getting Architecture Patch

Carlo Bertolli cbertol at us.ibm.com
Thu Jan 15 19:42:32 PST 2015


Hi Jonathan

Apologies for replying late. I believe this new version of the patch
follows your guidelines (attached). I produced it starting from the last
commits happened about a week ago or so.

C Bergström: I tested this patch on both ppc64, ppc64le, and x86_64. I do
not have any available ARM 64 architecture - can you please try it and let
me know if I broke anything?


Thanks

-- Carlo



(See attached file: ppc64le-support.patch)






From:	"Peyton, Jonathan L" <jonathan.l.peyton at intel.com>
To:	Carlo Bertolli/Watson/IBM at IBMUS, "Cownie, James H"
            <james.h.cownie at intel.com>
Cc:	C Bergström <cbergstrom at pathscale.com>,
            "openmp-dev at dcs-maillist2.engr.illinois.edu"
            <openmp-dev at dcs-maillist2.engr.illinois.edu>
Date:	01/06/2015 04:03 PM
Subject:	RE: Getting Architecture Patch



Carlo,

“
I tried your patch on both p7 and p8 (le).

It works on p7, but I get the following error on p8:

CMake Error at cmake/HelperFunctions.cmake:37 (message):
  check_variable(): arch = ppc64le is unknown

That string name ("ppc64le") is the default for p8 machines and we would
like it to be distinguished from the p7 one ("ppc64") in iomp.
“

Your patch has these lines in it:
-set(arch_possible_values        32e 32 arm ppc64)
+set(arch_possible_values        32e 32 arm ppc64 ppc64le)
So the problem above should be taken care of by this.  The idea was to have
my get_arch patch be independent of the recent architecture patches so that
the commits can be in a logical order.
Ideally, the get_arch patch will be first, then the architecture ports are
based off of that.

Some comments below on the ppc64le patch…

---Comment #1---
Below, the KMP_ARCH_PPC macro should be removed, then undelete these two
lines:
-#  undef KMP_ARCH_PPC64
-#  define KMP_ARCH_PPC64 1
This way, there is generic KMP_ARCH_PPC64 and then the more specific
KMP_ARCH_PPC64BE and KMP_ARCH_PPC64LE macros.
I think Jim wanted to get rid of the KMP_ARCH_PPC anyways because it might
confuse between 32-bit and 64-bit.

--- a/runtime/src/kmp_os.h
+++ b/runtime/src/kmp_os.h
@@ -75,7 +75,10 @@

#define KMP_ARCH_X86        0
#define KMP_ARCH_X86_64        0
-#define KMP_ARCH_PPC64      0
+#define KMP_ARCH_PPC64_BE   0
+#define KMP_ARCH_PPC64_LE   0
+
+#define KMP_ARCH_PPC (KMP_ARCH_PPC64_LE || KMP_ARCH_PPC64_BE)

#ifdef _WIN32
# undef KMP_OS_WINDOWS
@@ -116,7 +119,7 @@
# define KMP_OS_UNIX 1
#endif

-#if (KMP_OS_LINUX || KMP_OS_WINDOWS) && !KMP_OS_CNK && !KMP_ARCH_PPC64
+#if (KMP_OS_LINUX || KMP_OS_WINDOWS) && !KMP_OS_CNK && !KMP_ARCH_PPC
# define KMP_AFFINITY_SUPPORTED 1
#else
# define KMP_AFFINITY_SUPPORTED 0
@@ -140,8 +143,13 @@
#  undef KMP_ARCH_X86
#  define KMP_ARCH_X86 1
# elif defined __powerpc64__
-#  undef KMP_ARCH_PPC64
-#  define KMP_ARCH_PPC64 1
+#  if defined __LITTLE_ENDIAN__
+#   undef KMP_ARCH_PPC64_LE
+#   define KMP_ARCH_PPC64_LE 1
+#  else
+#   undef KMP_ARCH_PPC64_BE
+#   define KMP_ARCH_PPC64_BE 1
+#  endif
# endif
#endif

In most places it seems there is already KMP_ARCH_PPC64.  So just eliminate
the changes that look similar to this:
-#if KMP_ARCH_PPC64
+#if KMP_ARCH_PPC

---Comment #2---
It seems that there needs to be additions to the file
/runtime/tools/src/common-defs.mk, because inside this file
Is where the architecture/platform checks are done.  It should look
something like this:
os       := $(call check_variable,os,lin lrb mac win)
-arch     := $(call check_variable,arch,32 32e 64 arm ppc64)
+arch     := $(call check_variable,arch,32 32e 64 arm ppc64 ppc64le)
platform := $(os)_$(arch)
-platform := $(call check_variable,platform,lin_32 lin_32e lin_64 lin_arm
lrb_32e mac_32 mac_32e win_32 win_32e win_64 lin_ppc64)
+platform := $(call check_variable,platform,lin_32 lin_32e lin_64 lin_arm
lrb_32e mac_32 mac_32e win_32 win_32e win_64 lin_ppc64 lin_ppc64le)
# oa-opts means "os and arch options". They are passed to almost all perl
scripts.
oa-opts  := --os=$(os) --arch=$(arch)
You will also have to modify the legal_arch variable (also in
common-defs.mk) of nested if’s to include ppc64le.

Also, this means you can get rid of the ppc64be value in these
conditionals.
--- a/runtime/src/makefile.mk
+++ b/runtime/src/makefile.mk
@@ -491,7 +491,8 @@ cpp-flags += -D KMP_LIBRARY_FILE=\"$(lib_file)\"
cpp-flags += -D KMP_VERSION_MAJOR=$(VERSION)

# customize ppc64 cache line size to 128, 64 otherwise
-ifeq "$(arch)" "ppc64"
+#ifeq "$(arch)" "ppc64"
+ ifneq (,$(filter "$(arch)","ppc64be" "ppc64le" "ppc64"))
    cpp-flags += -D CACHE_LINE=128
else
    cpp-flags += -D CACHE_LINE=64
@@ -503,7 +504,7 @@ cpp-flags += -D KMP_ASM_INTRINS
cpp-flags += -D KMP_USE_INTERNODE_ALIGNMENT=0
# Linux and MIC compile with version symbols
ifneq "$(filter lin lrb,$(os))" ""
-ifeq "$(filter ppc64,$(arch))" ""
+ifneq (,$(filter "$(arch)","ppc64be" "ppc64le" "ppc64"))
     cpp-flags += -D KMP_USE_VERSION_SYMBOLS
endif
endif

-- Johnny

From: Carlo Bertolli [mailto:cbertol at us.ibm.com]
Sent: Tuesday, December 23, 2014 12:50 PM
To: Peyton, Jonathan L; Cownie, James H
Cc: C Bergström; openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: RE: Getting Architecture Patch



Hi

Attached a patch that includes both Jonathan's own (cmake auto architecture
detection) and some additional fix-ups by me to allow running on ppc64le
architectures (Power 8).
My part of the patch follows suggestions made by Jim Cownie regarding the
macro definition.
This also goes in the direction stated by Jim in one of his last e-mails
related to a re-designing of architecture macros.

(See attached file: cmake_auto_arch_and_ppc64.patch)

Let me know if you prefer a patch as diffed from Jonathan's one.

Thanks

-- Carlo


Inactive hide details for "Peyton, Jonathan L" ---12/17/2014 07:28:07
PM---New patch is attached. 1) No and no.  You can’t use"Peyton, Jonathan
L" ---12/17/2014 07:28:07 PM---New patch is attached. 1) No and no.  You
can’t use #warn or #warning or the #pragma ways of doing a

From: "Peyton, Jonathan L" <jonathan.l.peyton at intel.com>
To: C Bergström <cbergstrom at pathscale.com>, Carlo Bertolli/Watson/IBM at IBMUS
Cc: "openmp-dev at dcs-maillist2.engr.illinois.edu" <
openmp-dev at dcs-maillist2.engr.illinois.edu>
Date: 12/17/2014 07:28 PM
Subject: RE: Getting Architecture Patch




New patch is attached.

1) No and no.  You can’t use #warn or #warning or the #pragma ways of doing
a preprocessor warn/message/info because it isn’t standard (MSVC doesn’t
have either #warn or #warning), #error is standard.  Also, you can’t rely
on running a small program, and EVERYTHING I’ve seen emphasizes this point.
You can link but not actually run.  There is a specific reason,
cross-compiling.  For example, if you relied on printf and are
cross-compiling arm on an x86 host, then it won’t run on the x86 host
system.

Since you asked what others do, I had looked at what CMake does
specifically when detecting the compiler:
* Creates and links a small executable that has a main() function in it
with a few global variables defined like
     char cmake_compiler_name[] = “COMPILER_NAME=[gcc]”;
     char cmake_compiler_version_major[] = “COMPILER_VERISON_MAJOR=
[00000004]”
     char cmake_compiler_version_minor[] = “COMPILER_VERISON_MINOR=
[00000008]”
     etc.
     These variables are set through an extensive search through compiler
predefined macros and some clever preprocessing macros/tricks.
* Searches through the executable via regex to find something like
“COMPILER_NAME=[gcc]” and extracts gcc, then 4, then 8 etc.
* An aside: because not every compiler produces a.out as default executable
and not every compiler has –o flag to redirect output, it searches every
file that was produced in the temporary work space besides the temporary.c
file.
If you look at how LLVM does it, just look at GetHostTriple.cmake in
llvm_root/cmake/modules/GetHostTriple.cmake.  It calls config.guess on
non-Windows platforms J.

2) I personally don’t know about his one.  32e is embedded in lots of
places including our internal tools and commercial product.  I can check
with everybody else of course and see what they think, but you would have
to wait till after the holiday season for anything to happen about it.

3) I did this in the new patch along with ppc64le

-- Johnny

From: C Bergström [mailto:cbergstrom at pathscale.com]
Sent: Wednesday, December 17, 2014 1:24 PM
To: Carlo Bertolli
Cc: Peyton, Jonathan L; openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: Re: Getting Architecture Patch

3 nits

1) I think you can change it to #warn and avoid the compiler failure
(please see what others are doing with similar approach and let me know).
There's also possibility of actually linking and doing a printf which would
avoid the whole regex mess.. Is there a strong reason for #error? (maybe
faster?)

2) Would anyone @Intel strongly oppose a global rename of 32e to something
which is more commonly used (x86_64, amd64 /* joking */ or x8664)

3) Can you include ARM64

Thanks

On Thu, Dec 18, 2014 at 1:53 AM, Carlo Bertolli <cbertol at us.ibm.com> wrote:

      Hi Johnny


      Thanks for this patch. I will give it a try on our machines and let
      you know.


      Cheers

      -- Carlo


      Inactive hide details for "Peyton, Jonathan L" ---12/17/2014 01:18:14
      PM---Due to recent aarch64 and ppc64le port patches that "Peyton,
      Jonathan L" ---12/17/2014 01:18:14 PM---Due to recent aarch64 and
      ppc64le port patches that have been sent, I have finished up this bit
      of c

      From: "Peyton, Jonathan L" <jonathan.l.peyton at intel.com>
      To: "openmp-dev at dcs-maillist2.engr.illinois.edu" <
      openmp-dev at dcs-maillist2.engr.illinois.edu>
      Cc: Carlo Bertolli/Watson/IBM at IBMUS, C Bergström <
      cbergstrom at pathscale.com>
      Date: 12/17/2014 01:18 PM
      Subject: Getting Architecture Patch





      Due to recent aarch64 and ppc64le port patches that have been sent, I
      have finished up this bit of code for CMake which detects the
      architecture by probing the compiler.

      Carlo and C. Bergström can you two look at this patch and see if it
      adequately solves the detecting architecture problem for you both and
      add the proper macro into the GetArchitecture.cmake (this will make
      sense once you look at the patch)?  I would like this to be committed
      before either port so both ports can use this new patch.

      -- Johnny
      [attachment "cmake_get_arch.patch" deleted by Carlo
      Bertolli/Watson/IBM] [attachment "cmake_get_arch_v2.patch" deleted by
      Carlo Bertolli/Watson/IBM]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20150115/2710bf5e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20150115/2710bf5e/attachment.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ppc64le-support.patch
Type: application/octet-stream
Size: 10135 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20150115/2710bf5e/attachment.obj>


More information about the Openmp-dev mailing list