[llvm] r313770 - [cmake] Add an option to build llvm with IR PGO
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 8 13:13:04 PST 2017
Vedant Kumar <vsk at apple.com> writes:
>> On Nov 8, 2017, at 10:46 AM, Justin Bogner <mail at justinbogner.com> wrote:
>>
>> Vedant Kumar <vsk at apple.com> writes:
>>>> On Nov 7, 2017, at 5:22 PM, Justin Bogner <mail at justinbogner.com> wrote:
>>>>
>>>> Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> writes:
>>>>> Author: vedantk
>>>>> Date: Wed Sep 20 10:16:01 2017
>>>>> New Revision: 313770
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=313770&view=rev
>>>>> Log:
>>>>> [cmake] Add an option to build llvm with IR PGO
>>>>>
>>>>> This adds an LLVM_ENABLE_IR_PGO option to enable building llvm and its
>>>>> tools with IR PGO instrumentation.
>>>>>
>>>>> Usage: -DLLVM_BUILD_INSTRUMENTED=On -DLLVM_ENABLE_IR_PGO=On (both
>>>>> options must be enabled)
>>>>
>>>> Why not make LLVM_BUILD_INSTRUMENTED a string like "FE" or "IR" instead
>>>> of a bool?
>>>
>>> I didn't want to break setups which set LLVM_BUILD_INSTRUMENTED:BOOL.
>>
>> See how LLVM_ENABLE_LTO handled that - it treats bools in that spot as
>> the old way and then handles the explicit options the new way. If you
>> think that overloads the flag too much that's fine though.
>
> Thanks. I think it'd be nice to have one option to control PGO instrumentation.
>
> How does this look?
>
> - This deprecates LLVM_ENABLE_IR_PGO but keeps it around for now.
> - Errors out when LLVM_BUILD_INSTRUMENTED and LLVM_BUILD_INSTRUMENTED_COVERAGE
> are both set.
This is pretty much exactly what I was thinking. LGTM.
> commit 11e5df29a5a73b8465e39cf78bd3a7e6de403d0a (HEAD -> master)
> Author: Vedant Kumar <vsk at apple.com>
> Date: Wed Nov 8 11:17:35 2017
>
> [cmake] Allow LLVM_BUILD_INSTRUMENTED to be set to IR or Frontend
>
> - This deprecates LLVM_ENABLE_IR_PGO but keeps it around for now.
> - Errors out when LLVM_BUILD_INSTRUMENTED and LLVM_BUILD_INSTRUMENTED_COVERAGE
> are both set.
>
> diff --git a/cmake/modules/HandleLLVMOptions.cmake b/cmake/modules/HandleLLVMOptions.cmake
> index cf1ece24848..c5390371845 100644
> --- a/cmake/modules/HandleLLVMOptions.cmake
> +++ b/cmake/modules/HandleLLVMOptions.cmake
> @@ -738,14 +738,15 @@ if(LLVM_ENABLE_EH AND NOT LLVM_ENABLE_RTTI)
> message(FATAL_ERROR "Exception handling requires RTTI. You must set LLVM_ENABLE_RTTI to ON")
> endif()
>
> -option(LLVM_ENABLE_IR_PGO "Build LLVM and tools with IR PGO instrumentation (experimental)" Off)
> +option(LLVM_ENABLE_IR_PGO "Build LLVM and tools with IR PGO instrumentation (deprecated)" Off)
> mark_as_advanced(LLVM_ENABLE_IR_PGO)
>
> -option(LLVM_BUILD_INSTRUMENTED "Build LLVM and tools with PGO instrumentation" Off)
> +set(LLVM_BUILD_INSTRUMENTED OFF CACHE STRING "Build LLVM and tools with PGO instrumentation. May be specified as IR or Frontend")
> mark_as_advanced(LLVM_BUILD_INSTRUMENTED)
> +string(TOUPPER "${LLVM_BUILD_INSTRUMENTED}" uppercase_LLVM_BUILD_INSTRUMENTED)
>
> if (LLVM_BUILD_INSTRUMENTED)
> - if (LLVM_ENABLE_IR_PGO)
> + if (LLVM_ENABLE_IR_PGO OR uppercase_LLVM_BUILD_INSTRUMENTED STREQUAL "IR")
> append("-fprofile-generate='${LLVM_PROFILE_DATA_DIR}'"
> CMAKE_CXX_FLAGS
> CMAKE_C_FLAGS
> @@ -768,6 +769,10 @@ append_if(LLVM_BUILD_INSTRUMENTED_COVERAGE "-fprofile-instr-generate='${LLVM_PRO
> CMAKE_EXE_LINKER_FLAGS
> CMAKE_SHARED_LINKER_FLAGS)
>
> +if (LLVM_BUILD_INSTRUMENTED AND LLVM_BUILD_INSTRUMENTED_COVERAGE)
> + message(FATAL_ERROR "LLVM_BUILD_INSTRUMENTED and LLVM_BUILD_INSTRUMENTED_COVERAGE cannot both be specified")
> +endif()
> +
> if(LLVM_ENABLE_LTO AND LLVM_ON_WIN32 AND NOT LINKER_IS_LLD_LINK)
> message(FATAL_ERROR "When compiling for Windows, LLVM_ENABLE_LTO requires using lld as the linker (point CMAKE_LINKER at lld-link.exe)")
> endif()
>
>
>
> vedant
>
>
>>
>>>> There's some prior art for that approach with how the
>>>> LLVM_ENABLE_LTO flag works. Having to specify two parameters for this
>>>> (and the fact that LLVM_ENABLE_IR_PGO is meaningless without the other)
>>>> is awkward.
>>>
>>> It makes sense to make LLVM_ENABLE_IR_PGO meaningful by itself. We
>>> just need to update the PGO cmake cache accordingly.
>>>
>>> vedant
>>>
>>>>
>>>>> Differential Revision: https://reviews.llvm.org/D38066
>>>>>
>>>>> Modified:
>>>>> llvm/trunk/CMakeLists.txt
>>>>> llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
>>>>>
>>>>> Modified: llvm/trunk/CMakeLists.txt
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/CMakeLists.txt?rev=313770&r1=313769&r2=313770&view=diff
>>>>> ==============================================================================
>>>>>
>>>>> --- llvm/trunk/CMakeLists.txt (original)
>>>>> +++ llvm/trunk/CMakeLists.txt Wed Sep 20 10:16:01 2017
>>>>> @@ -548,7 +548,8 @@ else()
>>>>> set(LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION FALSE CACHE INTERNAL "For Visual Studio 2013, manually copy natvis files to Documents\\Visual Studio 2013\\Visualizers" FORCE)
>>>>> endif()
>>>>>
>>>>> -if (LLVM_BUILD_INSTRUMENTED OR LLVM_BUILD_INSTRUMENTED_COVERAGE)
>>>>> +if (LLVM_BUILD_INSTRUMENTED OR LLVM_BUILD_INSTRUMENTED_COVERAGE OR
>>>>> + LLVM_ENABLE_IR_PGO)
>>>>> if(NOT LLVM_PROFILE_MERGE_POOL_SIZE)
>>>>> # A pool size of 1-2 is probably sufficient on a SSD. 3-4 should be fine
>>>>> # for spining disks. Anything higher may only help on slower mediums.
>>>>> @@ -556,10 +557,9 @@ if (LLVM_BUILD_INSTRUMENTED OR LLVM_BUIL
>>>>> endif()
>>>>> if(NOT LLVM_PROFILE_FILE_PATTERN)
>>>>> if(NOT LLVM_PROFILE_DATA_DIR)
>>>>> - file(TO_NATIVE_PATH "${LLVM_BINARY_DIR}/profiles/%${LLVM_PROFILE_MERGE_POOL_SIZE}m.profraw" LLVM_PROFILE_FILE_PATTERN)
>>>>> - else()
>>>>> - file(TO_NATIVE_PATH "${LLVM_PROFILE_DATA_DIR}/%${LLVM_PROFILE_MERGE_POOL_SIZE}m.profraw" LLVM_PROFILE_FILE_PATTERN)
>>>>> + file(TO_NATIVE_PATH "${LLVM_BINARY_DIR}/profiles" LLVM_PROFILE_DATA_DIR)
>>>>> endif()
>>>>> + file(TO_NATIVE_PATH "${LLVM_PROFILE_DATA_DIR}/%${LLVM_PROFILE_MERGE_POOL_SIZE}m.profraw" LLVM_PROFILE_FILE_PATTERN)
>>>>> endif()
>>>>> endif()
>>>>>
>>>>>
>>>>> Modified: llvm/trunk/cmake/modules/HandleLLVMOptions.cmake
>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/HandleLLVMOptions.cmake?rev=313770&r1=313769&r2=313770&view=diff
>>>>> ==============================================================================
>>>>> --- llvm/trunk/cmake/modules/HandleLLVMOptions.cmake (original)
>>>>> +++ llvm/trunk/cmake/modules/HandleLLVMOptions.cmake Wed Sep 20 10:16:01 2017
>>>>> @@ -730,13 +730,27 @@ if(LLVM_ENABLE_EH AND NOT LLVM_ENABLE_RT
>>>>> message(FATAL_ERROR "Exception handling requires RTTI. You must set LLVM_ENABLE_RTTI to ON")
>>>>> endif()
>>>>>
>>>>> +option(LLVM_ENABLE_IR_PGO "Build LLVM and tools with IR PGO instrumentation (experimental)" Off)
>>>>> +mark_as_advanced(LLVM_ENABLE_IR_PGO)
>>>>> +
>>>>> option(LLVM_BUILD_INSTRUMENTED "Build LLVM and tools with PGO instrumentation" Off)
>>>>> mark_as_advanced(LLVM_BUILD_INSTRUMENTED)
>>>>> -append_if(LLVM_BUILD_INSTRUMENTED "-fprofile-instr-generate='${LLVM_PROFILE_FILE_PATTERN}'"
>>>>> - CMAKE_CXX_FLAGS
>>>>> - CMAKE_C_FLAGS
>>>>> - CMAKE_EXE_LINKER_FLAGS
>>>>> - CMAKE_SHARED_LINKER_FLAGS)
>>>>> +
>>>>> +if (LLVM_BUILD_INSTRUMENTED)
>>>>> + if (LLVM_ENABLE_IR_PGO)
>>>>> + append("-fprofile-generate='${LLVM_PROFILE_DATA_DIR}'"
>>>>> + CMAKE_CXX_FLAGS
>>>>> + CMAKE_C_FLAGS
>>>>> + CMAKE_EXE_LINKER_FLAGS
>>>>> + CMAKE_SHARED_LINKER_FLAGS)
>>>>> + else()
>>>>> + append("-fprofile-instr-generate='${LLVM_PROFILE_FILE_PATTERN}'"
>>>>> + CMAKE_CXX_FLAGS
>>>>> + CMAKE_C_FLAGS
>>>>> + CMAKE_EXE_LINKER_FLAGS
>>>>> + CMAKE_SHARED_LINKER_FLAGS)
>>>>> + endif()
>>>>> +endif()
>>>>>
>>>>> option(LLVM_BUILD_INSTRUMENTED_COVERAGE "Build LLVM and tools with Code Coverage instrumentation" Off)
>>>>> mark_as_advanced(LLVM_BUILD_INSTRUMENTED_COVERAGE)
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list