This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Add configuration dirs as potential locations for llvm-lit and llvm-tblgen in standalone builds
ClosedPublic

Authored by sgraenitz on Jun 4 2019, 1:49 PM.

Details

Summary

If the provided LLVM build-tree used a multi-configuration generator like Xcode, LLVM_TOOLS_BINARY_DIR will have a generator-specific placeholder to express CMAKE_CFG_INTDIR. Thus llvm-lit and llvm-tblgen won't be found.
D62878 exports the actual configuration types so we can fix the path and add them to the search paths for find_program().

Diff Detail

Repository
rL LLVM

Event Timeline

sgraenitz created this revision.Jun 4 2019, 1:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2019, 1:49 PM
Herald added a subscriber: mgorny. · View Herald Transcript
This revision is now accepted and ready to land.Jun 4 2019, 1:52 PM

IIUC Clang uses the same mechanism. Moving append_configuration_directories() to AddLLVM.cmake could help there too. We may consider it for the future once D62859 is through.

xiaobai accepted this revision.Jun 4 2019, 2:06 PM

Seems fine to me

sgraenitz added inline comments.Jun 4 2019, 2:14 PM
lldb/cmake/modules/LLDBStandalone.cmake
67 ↗(On Diff #203014)

How is this working on Windows anyway? It should be llvm-tblgen.exe there right?

lldb/cmake/modules/LLDBStandalone.cmake
67 ↗(On Diff #203014)

Ah, good point. You should use llvm-tblgen${HOST_EXECUTABLE_SUFFIX} as the name. That will add the suffix when required.

sgraenitz updated this revision to Diff 203030.Jun 4 2019, 2:45 PM

Add CMAKE_EXECUTABLE_SUFFIX when searching for llvm-tblgen

sgraenitz marked 3 inline comments as done.Jun 4 2019, 2:47 PM
sgraenitz added inline comments.
lldb/cmake/modules/LLDBStandalone.cmake
67 ↗(On Diff #203014)

Ok added the suffix. I can imaging CMake would do it automatically if necessary, but it's not documented.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2019, 1:29 AM