Page MenuHomePhabricator

[cmake] Use relative cmake binary dir for processing pass plugins
ClosedPublic

Authored by alan-baker on Jan 2 2020, 1:58 PM.

Details

Summary

https://reviews.llvm.org/D61446 introduced a new function to process pass plugins that used CMAKE_BINARY_DIR. This is problematic when LLVM is a subproject. Instead use CMAKE_CURRENT_BINARY_DIR to get the right relative directory for cmake.

Diff Detail

Event Timeline

alan-baker created this revision.Jan 2 2020, 1:58 PM
llvm/cmake/modules/AddLLVM.cmake
907

Why CMAKE_CURRENT_BINARY_DIR and not LLVM_BINARY_DIR as in line 903?

909

Same here.

I don't think CMAKE_CURRENT_BINARY_DIR will work. It is different for each subdirectory, such that multiple plugins create separate Extension.def.tmp instead of appending to a central one. LLVM_BINARY_DIR might be the better choice.

alan-baker marked an inline comment as done.Jan 3 2020, 5:36 AM
alan-baker added inline comments.
llvm/cmake/modules/AddLLVM.cmake
907

That they didn't match was a left over mistake from testing. In my setups they evaluated to the same values. I'll switch all uses to LLVM_BINARY_DIR as Meinersbur suggested.

alan-baker updated this revision to Diff 236032.Jan 3 2020, 5:37 AM

Switch to use LLVM_BINARY_DIR instead of CMAKE_CURRENT_BINARY_DIR.

This revision is now accepted and ready to land.Jan 3 2020, 6:16 AM

I do not have commit access. Would it be possible for someone to commit it on my behalf? Thanks.

This revision was automatically updated to reflect the committed changes.

I changed CMAKE_CURRENT_BINARY_DIR to LLVM_BINARY_DIR in the commit description. Thank you for the patch.

This broke standalone build of clang. It's trying to write into /include (as in, root directory) now! My guess is that LLVM_BINARY_DIR is undefined at this point of standalone build, later it might be defined to point to directory where LLVM was installed which also wouldn't be correct.

 * ACCESS DENIED:  mkdir:        /include
CMake Error at /usr/lib/llvm/11/lib64/cmake/llvm/AddLLVM.cmake:894 (file):
  file failed to open for writing (No such file or directory):

    /include/llvm/Support/Extension.def.tmp
Call Stack (most recent call first):
  CMakeLists.txt:867 (process_llvm_pass_plugins)

@mgorny can you try with https://reviews.llvm.org/D74602 applied?

That seems to help. Thanks!