Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix weird CMake issue with custom LLVM #6519

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

LebedevRI
Copy link
Contributor

Without this, cmake fails with:

CMake Error in dependencies/llvm/CMakeLists.txt:
  Target "Halide_LLVM" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/repositories/halide/dependencies/llvm/"

  which is prefixed in the source directory.

LLVM_INCLUDE_DIRS there is /repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include,
and INTERFACE_INCLUDE_DIRECTORIES's property beforehand is `` (empty),
but after this line it suddenly becomes /repositories/halide/dependencies/llvm/$<BUILD_INTERFACE:/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include>.

This is quite obscure. I don't really understand what is going on,
but with the patch it builds fine.

Without this, cmake fails with:
```
CMake Error in dependencies/llvm/CMakeLists.txt:
  Target "Halide_LLVM" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/repositories/halide/dependencies/llvm/"

  which is prefixed in the source directory.
```

`LLVM_INCLUDE_DIRS` there is `/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include`,
and `INTERFACE_INCLUDE_DIRECTORIES`'s property beforehand is `` (empty),
but after this line it suddenly becomes `/repositories/halide/dependencies/llvm/$<BUILD_INTERFACE:/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include>`.

This is quite obscure. I don't really understand what is going on,
but with the patch it builds fine.
Copy link
Member

@alexreinking alexreinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I hate CMake's quoting rules...

@alexreinking alexreinking added backport me This change should be backported to release versions build Issues related to building Halide and with CI bug labels Dec 28, 2021
@LebedevRI
Copy link
Contributor Author

Thanks! I hate CMake's quoting rules...

Note: i really do not understand why quotes are needed here...

@alexreinking
Copy link
Member

Thanks! I hate CMake's quoting rules...

Note: i really do not understand why quotes are needed here...

Because it turns it into two arguments...

$<BUILD_INTERFACE:before-semicolon

and

after-semicolon>

And because CMake is inconsistent in how it treats this.

@alexreinking
Copy link
Member

Failures are unrelated. Tagging @steven-johnson for the llvm-14 fix.

@LebedevRI
Copy link
Contributor Author

Failures are unrelated. Tagging @steven-johnson for the llvm-14 fix.

Looks like the CI broke a week ago - https://buildbot.halide-lang.org/master/#/builders/173/builds/499 vs https://buildbot.halide-lang.org/master/#/builders/173/builds/500

cc @labrinea

ada028c32f47ca84a0b7be5d1ab4e3c943f859a3 is the first bad commit
commit ada028c32f47ca84a0b7be5d1ab4e3c943f859a3
Author: Alexandros Lamprineas <alexandros.lamprineas@arm.com>
Date:   Tue Dec 21 15:41:06 2021 +0000

    [AArch64] Add a tablegen pattern for UZP2.
    
    Converts concat_vectors((trunc (lshr)), (trunc (lshr))) to UZP2
    when the shift amount is half the width of the vector element.
    
    Differential Revision: https://reviews.llvm.org/D116021

 llvm/lib/Target/AArch64/AArch64InstrInfo.td     | 13 +++++++
 llvm/test/CodeGen/AArch64/arm64-uzp2-combine.ll | 47 +++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 llvm/test/CodeGen/AArch64/arm64-uzp2-combine.ll
$ git bisect log
git bisect start
# bad: [2950a0cbc981e07d319a8cd3f9efa8bdf71b4e5d] z
git bisect bad 2950a0cbc981e07d319a8cd3f9efa8bdf71b4e5d
# good: [ecb9d8e4e3c4623c2edcd2c50727103927d31508] [LICM] Hoist LOAD without sinking the STORE
git bisect good ecb9d8e4e3c4623c2edcd2c50727103927d31508
# good: [78b0f3701d441e887e92ff9feec4c226c67c334f] [HIPSPV][1/4] Refactor HIP tool chain
git bisect good 78b0f3701d441e887e92ff9feec4c226c67c334f
# good: [6969f8415df764a2b267827e2b8e88869668a654] [runtimes] Fix type on flag name in D115852
git bisect good 6969f8415df764a2b267827e2b8e88869668a654
# bad: [ad761f0c3961a09dffd3e7eaea82cd331db9184f] [mlir] Update BUILD.bazel to include `scf_tests`
git bisect bad ad761f0c3961a09dffd3e7eaea82cd331db9184f
# good: [8f85d5205da0310a40fcb92d1830ffa1de99d566] [tsan] Disable test from D115759 on Darwin
git bisect good 8f85d5205da0310a40fcb92d1830ffa1de99d566
# bad: [a3ea9052d6a16b13607046df6a324403fb51888d] [PowerPC] Do not increase cost for getUserCost with MMA types
git bisect bad a3ea9052d6a16b13607046df6a324403fb51888d
# good: [6e28b86cc629c351b1f5d238146af1b6f7ceb785] AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript
git bisect good 6e28b86cc629c351b1f5d238146af1b6f7ceb785
# bad: [00ec441253048f5e30540ea26bb0a28c42a5fc18] [Clang] debug-info-objname.cpp test: explictly encode a x86 target when using %clang_cl to avoid falling back to a native CPU triple.
git bisect bad 00ec441253048f5e30540ea26bb0a28c42a5fc18
# good: [d5b3cb07118ae41542ca2d687d3af0a0f10c0472] [libc++][NFC] Fix links to https://llvm.org/PR20183 in the tests
git bisect good d5b3cb07118ae41542ca2d687d3af0a0f10c0472
# good: [36ea9861e3b58a715434cc2ec5ee2ebf7053bbc9] [clang-format] Remove unnecessary qualifications. NFC.
git bisect good 36ea9861e3b58a715434cc2ec5ee2ebf7053bbc9
# bad: [ada028c32f47ca84a0b7be5d1ab4e3c943f859a3] [AArch64] Add a tablegen pattern for UZP2.
git bisect bad ada028c32f47ca84a0b7be5d1ab4e3c943f859a3
# good: [1a929525e86a20d0b3455a400d0dbed40b325a13] [clangd] Return error for textdocument/outgoingCalls rather than success
git bisect good 1a929525e86a20d0b3455a400d0dbed40b325a13
# first bad commit: [ada028c32f47ca84a0b7be5d1ab4e3c943f859a3] [AArch64] Add a tablegen pattern for UZP2.

@LebedevRI
Copy link
Contributor Author

LebedevRI commented Dec 29, 2021

Ok, i think there is no LLVM-side regression, we just need to look for the different instruction now, posted #6521.
Ok, a better snippet does suggest it's a LLVM regression: https://godbolt.org/z/EeqjGW58x

@alexreinking
Copy link
Member

I opened an issue with LLVM here: llvm/llvm-project#52919

@alexreinking alexreinking merged commit 9a530b1 into halide:master Dec 29, 2021
@LebedevRI LebedevRI deleted the fix-build-with-custom-llvm branch December 29, 2021 22:58
@LebedevRI
Copy link
Contributor Author

@alexreinking thank you!

alexreinking pushed a commit that referenced this pull request Jan 5, 2022
Without this, cmake fails with:
```
CMake Error in dependencies/llvm/CMakeLists.txt:
  Target "Halide_LLVM" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/repositories/halide/dependencies/llvm/"

  which is prefixed in the source directory.
```

`LLVM_INCLUDE_DIRS` there is `/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include`,
and `INTERFACE_INCLUDE_DIRECTORIES`'s property beforehand is `` (empty),
but after this line it suddenly becomes `/repositories/halide/dependencies/llvm/$<BUILD_INTERFACE:/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include>`.

This is quite obscure. I don't really understand what is going on,
but with the patch it builds fine.

(cherry picked from commit 9a530b1)
@alexreinking alexreinking removed the backport me This change should be backported to release versions label Jan 5, 2022
alexreinking pushed a commit that referenced this pull request Jan 6, 2022
Without this, cmake fails with:
```
CMake Error in dependencies/llvm/CMakeLists.txt:
  Target "Halide_LLVM" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/repositories/halide/dependencies/llvm/"

  which is prefixed in the source directory.
```

`LLVM_INCLUDE_DIRS` there is `/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include`,
and `INTERFACE_INCLUDE_DIRECTORIES`'s property beforehand is `` (empty),
but after this line it suddenly becomes `/repositories/halide/dependencies/llvm/$<BUILD_INTERFACE:/repositories/llvm-project/llvm/include;/builddirs/llvm-project/build-Clang13/include>`.

This is quite obscure. I don't really understand what is going on,
but with the patch it builds fine.

(cherry picked from commit 9a530b1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug build Issues related to building Halide and with CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants