Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PGI compiler toolchain. #1342

Merged
merged 9 commits into from
May 10, 2016
Merged

Add PGI compiler toolchain. #1342

merged 9 commits into from
May 10, 2016

Conversation

bartoldeman
Copy link
Contributor

@bartoldeman bartoldeman commented Aug 7, 2015

This adds a PGI compiler toolchain, see also pull request for easybuilders/easybuild-easyblocks#658

@hpcugentbot
Copy link

Automatic reply from Jenkins: Can I test this?

@@ -0,0 +1,71 @@
##
# Copyright 2015 Bart Oldeman
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, triple-licensing also works

@boegel
Copy link
Member

boegel commented Aug 7, 2015

@georgets, @geimer, @rjeschmi: since you have looked into PGI support as well, please review? is a draft of what you were working on available somewhere?

@boegel
Copy link
Member

boegel commented Aug 7, 2015

Jenkins: ok to test

@hpcugentbot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1945/
EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1945/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@rjeschmi
Copy link
Contributor

rjeschmi commented Aug 8, 2015

I didn't get started on anything, but I ahve a need for this so will try to test.

@hpcugentbot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1963/
EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/1963/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@rjeschmi
Copy link
Contributor

I have tested this and didn't have any problems.

@bartoldeman might want to rebase to develop

COMPILER_UNIQUE_OPTION_MAP = {
'i8': 'i8',
'r8': 'r8',
'optarch': '', # PGI by default generates code for the arch it is running on!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any way to disable this default behaviour, and specify the architecture to generate code for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target architecture can be specified using the -tp=<arch> option. PGI 14.1 supports the following architectures, newer versions may accept more (can't check atm):

    px              Generic x86 Processor
    k8              AMD64 Processor
    k8-64e          AMD64 Processor rev E or later, 64-bit mode
    barcelona       AMD Barcelona processor
    shanghai        AMD Shanghai processor
    istanbul        AMD Istanbul processor
    bulldozer       AMD Bulldozer processor
    piledriver      AMD Piledriver processor
    p7              Intel P7 Architecture (Pentium 4, Xeon, Centrino)
    core2           Intel Core-2 Architecture
    penryn          Intel Penryn Architecture
    nehalem         Intel Nehalem processor
    sandybridge     Intel SandyBridge processor
    haswell         Intel Haswell processor
    x64             Unified AMD/Intel mode; -tp=k8,p7

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geimer: thanks for the feedback!

It would be interesting to somehow enforce picking one of these in case the optarch toolchain option is set to False.
The default is True, which (apparently) corresponds to not specifying any specific flag with PGI, but every now and then False is specified explicitly, so we should not just ignore that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to support a custom toolchain option for PGI-based toolchains, something like target-platform?

Or, we should reuse optarch for this? If optarch is True, we don't add any specific flags, if it's False we complain to specify the name of a target platform instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

target-platform would be interesting for other compilers as well, however, I thought the recommended way of handling this is the --optarch=<arg> command-line option. Why not simply use a generic target like px or x64 if optarch is False? AFAIK, this is the default behavior of other compilers. For PGI you have to explicitly request it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that makes sense too, indeed. x64 seems like a decent default indeed, and people can always override via --optarch.

@bartoldeman: are you up for tackling this yourself, i.e. using -tp=x64 if optarch is set to False? See https://github.com/hpcugent/easybuild-framework/blob/master/easybuild/toolchains/compiler/craype.py#L100 for inspiration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done below in _set_compiler_flags, thanks @bartoldeman.

@boegel boegel added this to the v2.4.0 milestone Sep 14, 2015
@ocaisa
Copy link
Member

ocaisa commented Oct 23, 2015

I spoke to someone from NVIDIA yesterday and he mentioned that C++ support in PGI depends on the underlying GCC. I think it might be wise to build up the PGI toolchain in the same way as the intel toolchain (bintuils -> GCC + bintuils -> PGI) to side step problems from the underlying OS (like RedHAt shipping an ancient GCC and a binutils that doesn't support modern architectures)...I guess this doesn't matter here but it does in the associated easyconfigs.

@boegel
Copy link
Member

boegel commented Oct 23, 2015

@ocaisa: already done that way, see easybuilders/easybuild-easyconfigs#1833

@ocaisa
Copy link
Member

ocaisa commented Oct 23, 2015

eb --delete-lazy-comment

@boegel
Copy link
Member

boegel commented Oct 23, 2015

@ocaisa: please make that a separate PR ;)

@boegel boegel modified the milestones: v2.4.0, v2.5.0 Oct 28, 2015
@boegel boegel modified the milestones: v2.5.0, v2.6.0 Dec 14, 2015
@jgphpc jgphpc mentioned this pull request Jan 8, 2016
@boegel
Copy link
Member

boegel commented Jan 9, 2016

Other stuff is coming in that is built on top of this (cfr. #1537), so we should really try and get this merged.

@bartoldeman: we haven't heard from you in a while on this, are you still willing to follow up on this, or should we take it from here?

@bartoldeman
Copy link
Contributor Author

I rebased this PR and updated with the change from @jhein32 so pgf77 is used for F77.

@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2927/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@author: Bart Oldeman (McGill University, Calcul Quebec, Compute Canada)
"""

from easybuild.toolchains.compiler.pgi import Pgi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this import and use GCCcore as well? PGI compilers need an underlying GCC and I guess also binutils. Without GCCcore here any software installed there won't be available for PGI-based toolchains when using minimal toolchains.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this means just catching up with what is happening in easybuild... I put this in the same way as for iccifort.py.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bartoldeman!

@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2929/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2932/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@bartoldeman
Copy link
Contributor Author

Here is the story with numactl (I mentioned that at the conf call):
any program that you compile with pgcc (with or without -mp) is linked to libnuma:

$ pgcc hello.c
$ ldd a.out 
        linux-vdso.so.1 =>  (0x00007ffc6676b000)
        libpgmp.so => /sb/software/CentOS-6/eb/software/Core/PGI/16.3-GCC-4.9.3-2.25/linux86-64/16.3/lib/libpgmp.so (0x00002ae2cc3ca000)
        libnuma.so.1 => /usr/lib64/libnuma.so.1 (0x0000003b96e00000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x0000003b95a00000)
        libpgc.so => /sb/software/CentOS-6/eb/software/Core/PGI/16.3-GCC-4.9.3-2.25/linux86-64/16.3/lib/libpgc.so (0x00002ae2cc66c000)
        libm.so.6 => /lib64/libm.so.6 (0x0000003b95e00000)
        libc.so.6 => /lib64/libc.so.6 (0x0000003b95600000)
        /lib64/ld-linux-x86-64.so.2 (0x0000003b95200000)

If the PGI installer detects /usr/lib64/libnuma.so.1 it will use that, if not, it will symlink linux86-64/16.3/lib/libnuma.so and ....so.1 to linux86-64/16.3/lib/libpgnuma.so. EB has its own numactl however. Perhaps we need numactl built with GCCcore?

@valtandor
Copy link
Contributor

How is this toolchain coming along? Does it need some additional testing?

@boegel
Copy link
Member

boegel commented Apr 21, 2016

@valtandor things are slowly aligning to get this merged, see also easybuilders/easybuild#211

A main issue on my end is that I need to set up the license that was provided to me by PGI, for testing purposes. That's less straightforward than I was hoping it would be, but I'm getting there...

@boegel
Copy link
Member

boegel commented May 2, 2016

@bartoldeman do you mind updating this with latest develop, to trigger Travis testing your PR as well (we're working on moving from Jenkins to Travis, and I've enabled Travis tests as required to be able to merge PRs)?

Let me know if you need help with that, I can easily send you a PR for it and that's easier for you.

@hpcugentbot
Copy link

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/3009/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Member

boegel commented May 9, 2016

All green, so this is good to go...

I'd like to get easybuilders/easybuild-easyconfigs#2899 merge-ready first though.

It pretty much should be, but we've hit a snag with the module naming scheme @bartoldeman is using, causing the OpenMPI sanity check to fail. I'd like to figure that out first...

@ocaisa, @damianam: have you guys ever used PGI with ToolchainMNS, or another hierarchical module naming scheme?

@boegel
Copy link
Member

boegel commented May 10, 2016

#1754 fixes the bug that was holding this PR back, and is merged now, so this is good to go as well

Thanks a lot for your work on this @bartoldeman, and for your patience in following this up; and of course also thanks to everyone else who providing feedback and testing!

@boegel boegel merged commit e2eefd1 into easybuilders:develop May 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants