[llvm] r313770 - [cmake] Add an option to build llvm with IR PGO

Wed Nov 8 13:27:06 PST 2017

> On Nov 8, 2017, at 1:13 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> 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.

Done, r317725.

vedant

> 
>> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171108/d17c9208/attachment.html>