Page MenuHomePhabricator

[flang] Fix issue of flang/runtime/config.h not being found in out of tree builds
ClosedPublic

Authored by isuruf on Jun 5 2020, 8:44 AM.

Diff Detail

Event Timeline

isuruf created this revision.Jun 5 2020, 8:44 AM
Herald added a project: Restricted Project. · View Herald Transcript
isuruf added a project: Restricted Project.Jun 5 2020, 8:45 AM
isuruf retitled this revision from Fix issue of flang/runtime/config.h note bing found in out of tree builds to Fix issue of flang/runtime/config.h note being found in out of tree builds.Jun 5 2020, 8:46 AM
PeteSteinfeld retitled this revision from Fix issue of flang/runtime/config.h note being found in out of tree builds to [flang] Fix issue of flang/runtime/config.h note being found in out of tree builds.Jun 5 2020, 9:07 AM
isuruf retitled this revision from [flang] Fix issue of flang/runtime/config.h note being found in out of tree builds to [flang] Fix issue of flang/runtime/config.h not being found in out of tree builds.Jun 5 2020, 9:09 AM
PeteSteinfeld requested changes to this revision.Jun 5 2020, 9:32 AM

Thanks for working on this!

I tried your change in my out-of-tree build, and it didn't fix anything. When I do out-of-tree builds, I create a build directory in the flang directory and then call cmake using the following script:

#!/bin/bash
cmake -G Ninja \
  -DCMAKE_CXX_STANDARD=17 \
  -DLLVM_TARGETS_TO_BUILD=host \
  -DLLVM_LIT_ARGS=-v \
  -D LLVM_EXTERNAL_LIT=/mnt/c/GitHub/master/clean/build/bin/llvm-lit \
  -D LLVM_DIR=/mnt/c/GitHub/master/clean/install/lib/cmake/llvm \
  -D MLIR_DIR=/mnt/c/GitHub/master/clean/install/lib/cmake/mlir \
  -DCMAKE_BUILD_TYPE=Release ..

How do you do out-of-tree builds to make this work for you?

This revision now requires changes to proceed.Jun 5 2020, 9:32 AM

Thanks for working on this!

I tried your change in my out-of-tree build, and it didn't fix anything. When I do out-of-tree builds, I create a build directory in the flang directory and then call cmake using the following script:

#!/bin/bash
cmake -G Ninja \
  -DCMAKE_CXX_STANDARD=17 \
  -DLLVM_TARGETS_TO_BUILD=host \
  -DLLVM_LIT_ARGS=-v \
  -D LLVM_EXTERNAL_LIT=/mnt/c/GitHub/master/clean/build/bin/llvm-lit \
  -D LLVM_DIR=/mnt/c/GitHub/master/clean/install/lib/cmake/llvm \
  -D MLIR_DIR=/mnt/c/GitHub/master/clean/install/lib/cmake/mlir \
  -DCMAKE_BUILD_TYPE=Release ..

How do you do out-of-tree builds to make this work for you?

I'm seeing the same thing as Pete. CMAKE_CURRENT_BINARY_DIR is the right place to look for config.h and I see that it is added to the INCLUDE_DIRECTORIES property of FortranRuntime, but it's not getting added as a -I option to the compilation command.

isuruf updated this revision to Diff 268873.Jun 5 2020, 10:36 AM

Use include_directories instead of target_include_directories

I was doing a CMake in-tree build of a out-of-llvm-tree flang. I can reproduce with a CMake out-of-tree build of a out-of-llvm-tree flang and fixed it here.

tskeith accepted this revision.Jun 5 2020, 10:45 AM

Looks good now. Thanks for taking care of this.

PeteSteinfeld accepted this revision.Jun 5 2020, 11:04 AM

Looks good to me, too!

Thanks again for doing this.

This revision is now accepted and ready to land.Jun 5 2020, 11:04 AM
sscalpone accepted this revision.Jun 5 2020, 3:40 PM

@isuruf - OK to merge this?

Yes, please. Shall I merge this? I don't know what the protocol is in merging.

@isuruf I think you've done enough reviews on here that you should just get direct access yourself.
The protocol is to get commit access as described on this page: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
Then once a review is approved you can just directly commit the code to master.

I already have merge access, but haven't merged any flang commits till now. It's in now.

This revision was automatically updated to reflect the committed changes.