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

Wed Nov 8 11:21:55 PST 2017

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-build-instrumented.diff
Type: application/octet-stream
Size: 2153 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171108/67714c5d/attachment.obj>
-------------- next part --------------

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