Skip to content

Add standard ipo variant for CMakePackage#18374

Merged
alalazo merged 2 commits intospack:developfrom
omor1:features/cmake-ipo
Oct 21, 2020
Merged

Add standard ipo variant for CMakePackage#18374
alalazo merged 2 commits intospack:developfrom
omor1:features/cmake-ipo

Conversation

@omor1
Copy link
Copy Markdown
Contributor

@omor1 omor1 commented Aug 29, 2020

Due to CMake's handling of compilers, linkers, archivers, etc., it can be harder to enable IPO/LTO by simply manipulating CFLAGS, AR, RANLIB, etc. than in autotools-based projects. The variable CMAKE_INTERPROCEDURAL_OPTIMIZATION has existed since CMake 3.9 to allow compilation with IPO/LTO more easily.

This PR adds a standard ipo variant to CMake packages, defaulting to False, that allows users to control this variable. Enabling it possibly breaks some packages, but it defaults to off, so I don't think it should cause any problems.

Closes #18373.

Copy link
Copy Markdown
Member

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

I'll wait for a couple other maintainers to review.

@adamjstewart
Copy link
Copy Markdown
Member

The variable CMAKE_INTERPROCEDURAL_OPTIMIZATION has existed since CMake 3.9 to allow compilation with IPO/LTO more easily.

For older versions of CMake, will the flag be ignored, or could it cause the build to crash? We could also check the version:

if self.spec.satisfies('^cmake@3.9:'):
    args.append(define('CMAKE_INTERPROCEDURAL_OPTIMIZATION', ipo))

@omor1
Copy link
Copy Markdown
Contributor Author

omor1 commented Aug 29, 2020

For older versions of CMake, will the flag be ignored, or could it cause the build to crash? We could also check the version:

if self.spec.satisfies('^cmake@3.9:'):
    args.append(define('CMAKE_INTERPROCEDURAL_OPTIMIZATION', ipo))

I think it's just ignored, but it might spit out a warning that confuses users, so that's a good idea. We probably don't want to just ignore the user's variant if they have an incompatible version of CMake, too; let's add a conflict with CMake < 3.9.

@omor1 omor1 force-pushed the features/cmake-ipo branch from 2410cae to dc63b2d Compare August 29, 2020 20:45
* +ipo sets CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON
* Conflict between +ipo and ^cmake@:3.8
@omor1 omor1 force-pushed the features/cmake-ipo branch from dc63b2d to c28a061 Compare October 17, 2020 00:02
@adamjstewart
Copy link
Copy Markdown
Member

Ping @alalazo

@alalazo alalazo merged commit 1147220 into spack:develop Oct 21, 2020
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Oct 21, 2020

Thanks @omor1 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Standardized CMake Interprocedural Optimization Variant

4 participants