This is an archive of the discontinued LLVM Phabricator instance.

Part 1 of 2-part patch: Use a specified list of languages in cmake project() command.
ClosedPublic

Authored by dougk on Jun 24 2015, 1:32 PM.

Details

Summary

This is necessary to allow for explicit handling of '.S' files as distinct from '.c', which is necessary to allow different compilation options, which will eventually allow silencing some "-pedantic" warnings when compiling the assembly code of compiler-rt with gcc.

Diff Detail

Event Timeline

dougk updated this revision to Diff 28393.Jun 24 2015, 1:32 PM
dougk retitled this revision from to Part 1 of 2-part patch: Use a specified list of languages in cmake project() command..
dougk updated this object.
dougk edited the test plan for this revision. (Show Details)
dougk added a reviewer: chapuni.
dougk added a subscriber: Unknown Object (MLST).
rnk added a subscriber: rnk.Jul 27 2015, 1:42 PM
rnk added inline comments.
cmake/modules/HandleLLVMOptions.cmake
169

This looks like it adds assembler flags without checking if the assembler supports them. What flags need to come through here? -mcpu? I think the list of flags that we need to pass to the assembler is probably shorter than the list of flags we pass to the compiler, and we might want to special case things in that direction.

dougk added inline comments.Jul 27 2015, 4:51 PM
cmake/modules/HandleLLVMOptions.cmake
169

That point is in general valid - some flag might be an error for the assembler. But in practice, the only flag ever passed to add_flag_or_print_warning is '-fPIC' (in the same file, about 20 lines down).

In terms of deciding whether a flag is acceptable to the assembler, this is difficult.
There is no CheckASMSourceCompiles() function which would be the analogous thing to CheckCXXSourceCompiles, which is a cmake macro that tests whether any given flag will fail the compile step. The cmake documentation explicitly states that there is no test for whether the assembler "works". Writing such a test might be possible by first compiling a trivial C program into assembly, then running the assembler with the additional flag to see whether that flag causes failure. But I think we don't need it (yet). The bottom line is that we have to assume that 'fPIC' is accepted by the assembler whenever it is acceptable to the CXX compiler.

There is also the question of whether other flags need to go into CMAKE_ASM_FLAGS that are not added specifically in add_flag_or_print_warning. Visual inspection suggests not. Pretty much all of the flags that are added into CMAKE_C_FLAGS and CMAKE_CXX_FLAGS are '-f' and '-W' options, which have no relevance to assembly source.

rnk accepted this revision.Jul 27 2015, 5:28 PM
rnk added a reviewer: rnk.

lgtm

cmake/modules/HandleLLVMOptions.cmake
169

Nice. Let's do it then.

This revision is now accepted and ready to land.Jul 27 2015, 5:28 PM
This revision was automatically updated to reflect the committed changes.