This is an archive of the discontinued LLVM Phabricator instance.

[AIX] For XL, pick GCC-compatible std & default warning options
ClosedPublic

Authored by hubert.reinterpretcast on Aug 14 2019, 2:20 PM.

Details

Summary

LLVM now requires C++14. For IBM XL compilers with C++14 support, this can be done with the GCC-style options. The relevant block in the CMake file is split up into smaller parts as part of this patch to allow the common cases to be shared.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2019, 2:20 PM
jfb added a comment.Aug 14 2019, 2:29 PM

Can XL enable all the same flags which GCC can? There's a lot under that elseif! At that point, isn't XL GCC compatible?

In D66256#1630326, @jfb wrote:

Can XL enable all the same flags which GCC can?

It can except the modules stuff, which should would be detected via the CHECK_CXX_SOURCE_COMPILES.

There's a lot under that elseif!

Most of it is the modules stuff.

At that point, isn't XL GCC compatible?

It isn't, because the object-file generation options (e.g., -fno-unwind-tables) differ.

I guess I should say "default warning" options in the commit message, because the LLVM_ENABLE_WARNINGS and LLVM_ENABLE_PEDANTIC modes aren't enabled with this patch.

jfb added a comment.Aug 14 2019, 2:45 PM
In D66256#1630326, @jfb wrote:

Can XL enable all the same flags which GCC can?

It can except the modules stuff, which should would be detected via the CHECK_CXX_SOURCE_COMPILES.

There's a lot under that elseif!

Most of it is the modules stuff.

At that point, isn't XL GCC compatible?

It isn't, because the object-file generation options (e.g., -fno-unwind-tables) differ.

I guess I should say "default warning" options in the commit message, because the LLVM_ENABLE_WARNINGS and LLVM_ENABLE_PEDANTIC modes aren't enabled with this patch.

I'm worried because this entire section is for "GCC compatible command line stuff". If someone added something new in there, would it inadvertently break XL? Maybe it's better to factor out the bits you want to enable and leave the rest separate from XL?

In D66256#1630348, @jfb wrote:

I'm worried because this entire section is for "GCC compatible command line stuff". If someone added something new in there, would it inadvertently break XL? Maybe it's better to factor out the bits you want to enable and leave the rest separate from XL?

Good point. Will do.

Address comments: Split large GCC-compatible options block

hubert.reinterpretcast retitled this revision from [AIX] Use GCC-style -std and warning options for XL to [AIX] For XL, pick GCC-compatible std & default warning options.Aug 14 2019, 7:35 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)
xingxue accepted this revision.Aug 15 2019, 6:03 AM

LGTM.

This revision is now accepted and ready to land.Aug 15 2019, 6:03 AM
jfb accepted this revision.Aug 15 2019, 10:31 AM
This revision was automatically updated to reflect the committed changes.
daltenty added inline comments.Aug 16 2019, 8:46 AM
llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
435 ↗(On Diff #215482)

I'm not sure you needed to wrap everything in this else now, since everything inside is now guarded on LLVM_COMPILER_IS_GCC_COMPATIBLE and if (MSVC) LLVM_COMPILER_IS_GCC_COMPATIBLE=OFF

hubert.reinterpretcast marked 2 inline comments as done.Aug 18 2019, 3:29 PM
hubert.reinterpretcast added inline comments.
llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
435 ↗(On Diff #215482)

Thanks; committed NFC patch rL369221 to address this.