Hi all
The following patch is to support the PPC64LE (little endian) architecture.
Please note that in the source code I distinguished the two architectures with two different macros (KMP_ARCH_PPC64 and KMP_ARCH_PPC64LE).
However, the two macros are currently always used together and the compilation path is precisely the same.
I did this because I wanted to make sure that in the future we are able to distinguish them.
Please let me know your thoughts.
(See attached file: ppc64le_patch.txt)
Thanks
– Carlo
ppc64le_patch.txt (14.6 KB)
I don't think this is super important, but I would have done the following
KMP_ARCH_PPC64 == Things shared for both BE and LE
KMP_ARCH_PPC64BE == BE specific
KMP_ARCH_PPC64LE == LE specific
Currently in the patch I think you're using KMP_ARCH_PPC64 to mean BE, but
in the future if anything is common to both of them will you just use
KMP_ARCH_PPC64 || KMP_ARCH_PPC64LE or what's the plan? (They can't always
be mutually exclusive - can they?)
Hi,
Thanks for the suggestion - I actually like it.
Yes, the plan was:
-
If something is common between BE and LE PPC64 archs, then I would use KMP_ARCH_PPC64 || KMP_ARCH_PPC64LE.
-
If something only applies to PPC64 BE, then I would use KMP_ARCH_PPC64.
-
If something only applies to PPC64 LE, then I would use KMP_ARCH_PPC64LE.
If you feel like this is something critical, I will produce a new patch with the *BE and *LE macros. Otherwise, I will add it in the next patch I am going to produce.
Thanks
– Carlo
C Bergström —10/23/2014 02:33:22 AM—On Thu, Oct 23, 2014 at 9:50 AM, Carlo Bertolli cbertol@us.ibm.com wrote: > Hi all
It’s certainly not critical and a small amount of code churn in the next patch is fine.
This passes my review if someone wants to push it.
Thanks
I think it may be worth making the clean-up that Christopher suggested before we push this, unless there is real urgency here.
This patch is adding a lot of
#if KMP_ARCH_PPC64 || KMP_ARCH_PPC64LE
and similar changes, which would be unnecessary if we went to the other scheme.
So you would explicitly define the architecture in the scripts and so on as “ppc64be” or “ppc64le”, and the you would define
KMP_ARCH_PPC64LE or KMP_ARCH_PPC64BE in the compiler flags and have a
#define KMP_ARCH_PPC (KMP_ARCH_PPC64LE || KMP_ARCH_PPC64_BE)
in an appropriate place.
Then all the endian insensitive code would need no changes elsewhere.
I think that could really reduce the number of changes and late rreversions.
– Jim
James Cownie james.h.cownie@intel.com
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438
Jim
Apologies for this late answer - I had to put this on a side for some time.
Attached a new patch from your last commit to the trunk that follows the macro convention you suggested.
Please let me know of any problem with this.
(See attached file: ppc64_patch_2)
Thanks
– Carlo
"Cownie, James H" —10/24/2014 09:32:37 AM—I think it may be worth making the clean-up that Christopher suggested before we push this, unless t
ppc64_patch_2 (14.7 KB)
Apologies for this late answer - I had to put this on a side for some time.
No problem, what with SC14 and so on we’ve all been busy!
Please let me know of any problem with this.
You’ve made one change that I don’t like, in that AFAICS you renamed the KMP_ARCH_PPC64 to be KMP_ARCH_PPC.
Unless this really is for both 32 and 64 bit PPCs, I’d like to keep the 64 on there (in case you later do want a 32 bit variant).
