Skip to content

Update gcc easyblock to add enable-fat option#1336

Merged
boegel merged 5 commits intoeasybuilders:developfrom
raj76:patch-1
Feb 24, 2018
Merged

Update gcc easyblock to add enable-fat option#1336
boegel merged 5 commits intoeasybuilders:developfrom
raj76:patch-1

Conversation

@raj76
Copy link
Copy Markdown
Contributor

@raj76 raj76 commented Jan 5, 2018

This change will build gcc so it will run on multiple platforms. During stage2 build that gmp was being optimized for the processor on the build machine. The --enable-fat option to configure enables gmp to run on other platforms. This fix has been tested on our test cluster for building foss 2016b, 2017a, 2017b on AMD and Intel platforms.

This change will build gcc so it will run on multiple platforms. During stage2 build that gmp was being optimized for the processor on the build machine. The --enable-fat option to configure enables gmp to run on other platforms.
@boegel boegel added this to the 3.5.1 milestone Jan 7, 2018
@boegel boegel added the change label Jan 7, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Jan 7, 2018

Hmm, I'm not sure we should do this by default, since strictly speaking this introduces a performance regression for GCC compared to how we were installing it previously...

On the other hand, the performance of GCC is certainly not critical (as opposed to scientific software, where it is).

Thoughts on this @geimer, @wpoely86?

@raj76 On the EasyBuild mailing list you also mentioned similar issues with ISL? Is this change sufficient to make the GCC build work on multiple platforms?

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Jan 7, 2018

@raj76 According to the GMP docs, this option only works on x86 (cfr. https://gmplib.org/manual/Build-Options.html#Build-Options). While the docs could be out of date, we should definitely double-check that unconditionally adding this option doesn't cause trouble on POWER or ARM.

@boegel I have no clue where GMP is being used within GCC. I could imagine something like constant folding within the compiler, but quite likely also in the Fortran runtime libs, so there may indeed be a performance penalty. How severe it is is hard to judge, though. So maybe this should be implemented as a package-specific option which is disabled by default?

As far as I know, ISL (or PPL/CLooG for older versions of GCC) is used for the Graphite loop transformations in the optimizer. And I believe they have to be enabled explicitly. So this has to be taken into account when testing whether ISL needs something similar.

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Jan 8, 2018

I did some further research. Here are my findings:

  • GMP:
    If I interpret the configure.ac from GMP 6.1.2 correctly, using --enable-fat should be harmless (i.e., ignored) on non-x86 architectures -- though I haven't tested it yet. It disables all -march and -mtune options on x86, leaving a generic -O2 -fomit-frame-pointer and runtime-selection of optimized assembly. The compiler options can be overridden using CFLAGS though, to optimize the C code for the lowest common denominator.
    Using --enable-fat is also advised for package builds (cfr. https://gmplib.org/manual/Notes-for-Package-Builds.html).
  • MPFR:
    MPFR would pick up its compiler flags from the GMP build if it were built separately. So whatever you've done there would propagate. But as far as I can see, it is currently built as part of the GCC stage 3 build procedure using -g.
  • MPC:
    Same as MPFR.
  • ISL:
    ISL has it's own architecture detection code. But from what I can see in ISL 1.16.1, the newest architectures supported are 'nocona' for Intel and 'opteron' for AMD. On my Haswell laptop, it is actually unable to determine the architecture at all. So it probably doesn't matter that much (it's using GMP which most likely does the heavy lifting anyway). There is also an --enable-portable-binary option which only specifies -mtune in case it can detect the architecture and leaves -march alone.

I haven't checked PPL/CLooG, as it would only affect GCC 4.x anyway. Whoever is interested in this outdated stuff has to look into this.

My conclusion:
Passing --enable-fat to GMP and --enable-portable-binary to ISL should in theory be sufficient to get a portable GCC v5+, while still leaving the possibility to apply further site-specific optimizations via CFLAGS. However, this behavior should be configurable via a custom easyconfig parameter for the (probably rare) cases when this is needed, to not impose a performance impact on everyone else with just a single target platform.

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Jan 8, 2018

A quick check on my AArch64 box shows that passing --enable-fat to GMP's configure script is indeed silently ignored.

@raj76
Copy link
Copy Markdown
Contributor Author

raj76 commented Jan 8, 2018

@boegel Just setting --enable-fat option for GMP made gcc work on both AMD and Intel platforms.
I agree that making this a configurable option via custom easyconfig parameter would be better. I didn't know kow to do it and this was the easier option for me.

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Jan 9, 2018

@raj76 I'll take a look at the custom easyconfig option. Stay tuned...

BTW: Passing --enable-portable-binary to ISL doesn't have the documented effect :-( However, --without-gcc-arch can be used instead.

@geimer
Copy link
Copy Markdown
Contributor

geimer commented Jan 9, 2018

@raj76 Custom easyconfig parameter implemented in raj76#1

@boegel boegel modified the milestones: 3.5.1, 3.6.0 Jan 11, 2018
Make generic code generation configurable via a custom easyconfig parameter
@raj76
Copy link
Copy Markdown
Contributor Author

raj76 commented Jan 16, 2018

@geimer Thanks for adding the custom easyconfig parameter. It works as expected.

@easybuilders easybuilders deleted a comment from boegelbot Feb 22, 2018
Comment thread easybuild/easyblocks/g/gcc.py Outdated
cmd = "./configure --prefix=%s " % stage2prefix
cmd += "--with-pic --disable-shared --enable-cxx"
cmd += "--with-pic --disable-shared --enable-cxx "
if self.cfg['generic']:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@raj76 @geimer Should we also consider the use of --optarch=GENERIC to enable this?

from easybuild.tools.config import build_option
from easybuild.tools.toolchain.compiler import OPTARCH_GENERIC
...
            if build_option('optarch') == OPTARCH_GENERIC or self.cfg['generic']:

Having a dedicated easyconfig parameter makes sense, but maybe we should use None as default value.
That way we can allow people setting it to False to indicate that the generic build should not be done when --optarch=GENERIC is used. That's a bit of a weird use case, but I guess it's possible?

if build_option('optarch') == OPTARCH_GENERIC and self.cfg['generic'] != False:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am OK with using --optarch=GENERIC to enable this as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I made the proposed changes in raj76#2, please review/merge to update this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tested the updated gcc.py both with and without --optarch=GENERIC, works as intended.

also consider use of --optarch=generic to build GCC generically
@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 24, 2018

Thanks for looking into this @raj76 and @geimer!

@boegel boegel merged commit e6f1b5f into easybuilders:develop Feb 24, 2018
@raj76 raj76 deleted the patch-1 branch February 28, 2018 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants