[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

In D141581#4072692 , @kwk wrote:

> In D141581#4071617 , @thesamesam 
> wrote:
>
>> This is currently holding back further testing on our end which is 
>> concerning me a bit, especially as we approach the branch point. Could you 
>> revert this please if a fix isn't imminent? Thank you!
>
> @thesamesam what do you mean with "revert"? Just by looking at this 
> differential I cannot see that this patch has landed. Has it?

He was referring to reverting ac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf 
, i.e. the 
change that introduced the problem. However, FWICS reverting it is non-trivial 
and will probably cause quite a mess.

So, well, we're stuck for two weeks now being unable to test new LLVM versions, 
the branching point is approaching fast and it's quite likely we'll have to put 
a lot of effort afterwards to fix everything and request backporting our fixes 
to 16.x.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli updated this revision to Diff 491320.
fpetrogalli added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, sstefan1.
Herald added a project: LLVM.

@mgorny, I have updated the patch according to the suggestions from @tstellar 
in https://reviews.llvm.org/D141581#4069857.

I have used the script below to test the stand-alone build of clang. Indeed, 
the error related to the cmake configuration for RISCVTargetParserDefs has 
disappeared. However, there is an (unreltaed) error that does not allow me to 
test the full build:

  CMake Error at 
/Users/fpetrogalli/projects/cpu-defs/install/lib/cmake/llvm/LLVMExports.cmake:1222
 (set_target_properties):
The link interface of target "LLVMLineEditor" contains:
  
  LibEdit::LibEdit
  
but the target was not found.  Possible reasons include:
  
  * There is a typo in the target name.
  * A find_package call is missing for an IMPORTED target.
  * An ALIAS target is missing.
  
  Call Stack (most recent call first):

/Users/fpetrogalli/projects/cpu-defs/install/lib/cmake/llvm/LLVMConfig.cmake:329
 (include)
CMakeLists.txt:38 (find_package)

Script used for testing:

  % cat standalone.sh
  #!/bin/sh
  
  build_llvm=`pwd`/build-llvm
  build_clang=`pwd`/build-clang
  installprefix=`pwd`/install
  llvm=`pwd`/llvm-project
  mkdir -p $build_llvm
  mkdir -p $installprefix
  
  cmake -G Ninja -S $llvm/llvm -B $build_llvm \
-DLLVM_INSTALL_UTILS=ON \
-DCMAKE_INSTALL_PREFIX=$installprefix \
-DCMAKE_BUILD_TYPE=Release
  
  ninja -C $build_llvm install
  
  cmake -G Ninja -S $llvm/clang -B $build_clang \
-DLLVM_EXTERNAL_LIT=$build_llvm/utils/lit \
-DLLVM_ROOT=$installprefix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Driver/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in


Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -140,9 +140,9 @@
   @llvm_config_include_buildtree_only_exports@
 endif()
 
-# By creating intrinsics_gen, omp_gen and acc_gen here, subprojects that depend
-# on LLVM's tablegen-generated headers can always depend on this target whether
-# building in-tree with LLVM or not.
+# By creating the following targets here, subprojects that depend on
+# LLVM's tablegen-generated headers can always depend on this target
+# whether building in-tree with LLVM or not.
 if(NOT TARGET intrinsics_gen)
   add_custom_target(intrinsics_gen)
 endif()
@@ -152,6 +152,9 @@
 if(NOT TARGET acc_gen)
   add_custom_target(acc_gen)
 endif()
+if(NOT TARGET RISCVTargetParserTableGen)
+  add_custom_target(RISCVTargetParserTableGen)
+endif()
 
 set_property(GLOBAL PROPERTY LLVM_TARGETS_CONFIGURED On)
 include(${LLVM_CMAKE_DIR}/LLVM-Config.cmake)
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -93,7 +93,6 @@
 
   DEPENDS
   ClangDriverOptions
-  RISCVTargetParserTableGen
 
   LINK_LIBS
   clangBasic
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -110,7 +110,6 @@
 
   DEPENDS
   omp_gen
-  RISCVTargetParserTableGen
   )
 
 target_link_libraries(clangBasic


Index: llvm/cmake/modules/LLVMConfig.cmake.in
===
--- llvm/cmake/modules/LLVMConfig.cmake.in
+++ llvm/cmake/modules/LLVMConfig.cmake.in
@@ -140,9 +140,9 @@
   @llvm_config_include_buildtree_only_exports@
 endif()
 
-# By creating intrinsics_gen, omp_gen and acc_gen here, subprojects that depend
-# on LLVM's tablegen-generated headers can always depend on this target whether
-# building in-tree with LLVM or not.
+# By creating the following targets here, subprojects that depend on
+# LLVM's tablegen-generated headers can always depend on this target
+# whether building in-tree with LLVM or not.
 if(NOT TARGET intrinsics_gen)
   add_custom_target(intrinsics_gen)
 endif()
@@ -152,6 +152,9 @@
 if(NOT TARGET acc_gen)
   add_custom_target(acc_gen)
 endif()
+if(NOT TARGET RISCVTargetParserTableGen)
+  add_custom_target(RISCVTargetParserTableGen)
+endif()
 
 set_property(GLOBAL PROPERTY LLVM_TARGETS_CONFIGURED On)
 include(${LLVM_CMAKE_DIR}/LLVM-Config.cmake)
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -93,7 +93,6 @@
 
   DEPENDS
   ClangDriverOptions
-  RISCVTargetParserTableGen
 
   LINK_LIBS
   clangBasic
Index: clang/lib/Basic/CMakeLists.txt

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

In D141581#4071617 , @thesamesam 
wrote:

> This is currently holding back further testing on our end which is concerning 
> me a bit, especially as we approach the branch point. Could you revert this 
> please if a fix isn't imminent? Thank you!

@thesamesam what do you mean with "revert"? Just by looking at this 
differential I cannot see that this patch has landed. Has it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Sam James via Phabricator via cfe-commits]

thesamesam added a comment.

This is currently holding back further testing on our end which is concerning 
me a bit, especially as we approach the branch point. Could you revert this 
please if a fix isn't imminent? Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

@tstellar as you suspected, I had the following problem when building clang in 
standalone mode and your patch solved it for me. Thank you.

  -- Configuring done
  CMake Error at /usr/lib64/cmake/llvm/AddLLVM.cmake:536 (add_dependencies):
The dependency target "RISCVTargetParserTableGen" of target
"obj.clangBasic" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddClang.cmake:106 (llvm_add_library)
lib/Basic/CMakeLists.txt:40 (add_clang_library)
  
  
  CMake Error at /usr/lib64/cmake/llvm/AddLLVM.cmake:719 (add_dependencies):
The dependency target "RISCVTargetParserTableGen" of target "clangBasic"
does not exist.
  Call Stack (most recent call first):
cmake/modules/AddClang.cmake:106 (llvm_add_library)
lib/Basic/CMakeLists.txt:40 (add_clang_library)
  
  
  CMake Error at /usr/lib64/cmake/llvm/AddLLVM.cmake:536 (add_dependencies):
The dependency target "RISCVTargetParserTableGen" of target
"obj.clangDriver" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddClang.cmake:106 (llvm_add_library)
lib/Driver/CMakeLists.txt:17 (add_clang_library)
  
  
  CMake Error at /usr/lib64/cmake/llvm/AddLLVM.cmake:719 (add_dependencies):
The dependency target "RISCVTargetParserTableGen" of target "clangDriver"
does not exist.
  Call Stack (most recent call first):
cmake/modules/AddClang.cmake:106 (llvm_add_library)
lib/Driver/CMakeLists.txt:17 (add_clang_library)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

I spent some time looking at this.  Rather than changing the dependency from 
RISCVTargetParserTableGen to LLVMTargetParser, I think the correct fix is to 
handle RISCVTargetParserTableGen in llvm/cmake/modules/LLVMConfig.cmake.in.  
alongside the intrinsics_gen, omp_gen, and acc_gen targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4069123 , @tstellar wrote:

> I don't think this is the correct way to specify dependencies if it's just an 
> issue of the header being included before a generated file it needs has been 
> generated.  Are there other places in the code where a generated header file 
> is included by another header?



  % grep -r "RISCVTargetParser.h" *
  include/llvm/module.modulemap:header "TargetParser/RISCVTargetParser.h"
  lib/Target/RISCV/RISCVISelLowering.h:#include 
"llvm/TargetParser/RISCVTargetParser.h"
  lib/TargetParser/RISCVTargetParser.cpp:#include 
"llvm/TargetParser/RISCVTargetParser.h"

`RISCVTargetParser.h` is the one that references the generated file via :

  enum CPUKind : unsigned {
  #define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
  #define TUNE_PROC(ENUM, NAME) CK_##ENUM,
  #include "llvm/TargetParser/RISCVTargetParserDef.inc"
  };


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

I don't think this is the correct way to specify dependencies if it's just an 
issue of the header being included before a generated file it needs has been 
generated.  Are there other places in the code where a generated header file is 
included by another header?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4069078 , @tstellar wrote:

> So, what's the actual dependency here?  Do libBasic and libDriver just need 
> the header to be generated or does it actually need to link to the library?

Just the header.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4069053 , @tstellar wrote:

> [...]
> It's still not clear to me why LLVM_LINK_COMPONENTS does not work.  Is 
> LLVMTargetParser a library?

Yes, `LLVMTargetParser` is a library built located in `llvm/lib/TargetParser`. 
`LLVM_LINK_COMPONENTS` does not work if `clangBasic` or  `clangDriver` start 
building before `LLVMTargetParser`, because the header file 
`llvm/TargetParser/RISCVTargetParserDefs.inc` has not been generated yet. This 
didn't happen on my local build, but on some bot I had the following error 
reported even if `TargetParser` was added to `LLVM_LINK_COMPONENTS`.

/llvm-project/llvm/include/llvm/TargetParser/RISCVTargetParser.h:29:10:
 fatal error: llvm/TargetParser/RISCVTargetParserDef.inc: No such file or 
directory

  29 | #include "llvm/TargetParser/RISCVTargetParserDef.inc"
 |  ^~~~

By adding `LLVMTargetParser` to the `DEPENDS` of `clangBasic` and `clangDriver` 
it seems that the order of building LLVMTargetParser before any of the 
dependents is enforced.

To put in other words, it seems that just specifying` LINK_COMPONENTS = A` for 
a library `B` allows `A` and `B` to be built in parallel.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

So, what's the actual dependency here?  Do libBasic and libDriver just need the 
header to be generated or does it actually need to link to the library?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Is LLVMTargetParser a library

In D141581#4057177 , @fpetrogalli 
wrote:

> In D141581#4056825 , @barannikov88 
> wrote:
>
>> clangBasic and clangDriver already have a dependency on TargetParser (see 
>> LLVM_LINK_COMPONENTS at the beginning of corresponding files). Is that not 
>> enough?
>> Will it build if you just remove the additional dependency?
>
> No - if we just specify the dependency in `LLVM_LINK_COMPONENTS`, there are 
> build failures, as explained in the commit message of 
> https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf.

It's still not clear to me why LLVM_LINK_COMPONENTS does not work.  Is 
LLVMTargetParser a library?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi -has anybody any more concern on this change? I'd like to submit it as soon 
as possible to unlock @mgorny .

Francesco


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4056825 , @barannikov88 
wrote:

> clangBasic and clangDriver already have a dependency on TargetParser (see 
> LLVM_LINK_COMPONENTS at the beginning of corresponding files). Is that not 
> enough?
> Will it build if you just remove the additional dependency?

No - if we just specify the dependency in `LLVM_LINK_COMPONENTS`, there are 
build failures, as explained in the commit message of 
https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

clangBasic and clangDriver already have a dependency on TargetParser (see 
LLVM_LINK_COMPONENTS at the beginning of corresponding files). Is that not 
enough?
Will it build if you just remove the additional dependency?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D141581#4056671 , @fpetrogalli 
wrote:

> @barannikov88  - I am stuck with an incomplete explanation:

Ah, I see. The key point is that standalone build that depends on //installed// 
LLVM.
RISCVTargetParser is a custom target and it is not being installed, while 
LLVMTargetParser is a "real" target and gets installed. This is probably why 
changing the dependency fixes the issue.
The change looks good to me then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

@barannikov88  - Imam stuck with an incomplete explanation:

The issue raised by @mgorny is about stand-alone 
 builds. In this 
build configuration, clang is built separately from LLVM, which is first build 
and installed in a folder.

When clang is build as a stand-alone project, clang does not have access to the 
cmake configuration of LLVM. Therefore it knows nothing about the tablegen 
target `RISCVTargetParserTableGen`.

The's why we see the following error when building stand-alone clang on `main`:

  CMake Error at 
/Users/fpetrogalli/projects/cpu-defs/install/lib/cmake/llvm/AddLLVM.cmake:536 
(add_dependencies):
The dependency target "RISCVTargetParserTableGen" of target
"obj.clangBasic" does not exist.
  Call Stack (most recent call first):
cmake/modules/AddClang.cmake:106 (llvm_add_library)
lib/Basic/CMakeLists.txt:40 (add_clang_library)

I do not know why `LLVMTargetParser` (which is also specified in the cmake 
configuration of llvm) is instead recognised as a valid dependencies of clang 
on some LLVM library. Maybe because cmake looks in the prefix folder where LLVM 
was installed and finds the object file of the library?

FWIW, I was able to reproduce the issue reported by @mgorny via the following 
script when run on `main`:

  #!/bin/sh
  
  build_llvm=`pwd`/build-llvm
  build_clang=`pwd`/build-clang
  installprefix=`pwd`/install
  llvm=`pwd`/llvm-project
  mkdir -p $build_llvm
  mkdir -p $installprefix
  
  cmake -G Ninja -S $llvm/llvm -B $build_llvm \
-DLLVM_INSTALL_UTILS=ON \
-DCMAKE_INSTALL_PREFIX=$installprefix \
-DCMAKE_BUILD_TYPE=Release
  
  
  ninja -C $build_llvm install
  
  cmake -G Ninja -S $llvm/clang -B $build_clang \
-DLLVM_EXTERNAL_LIT=$build_llvm/utils/llvm-lit \
-DLLVM_ROOT=$installprefix

Given it was an issue that cmake itself was reporting (and not a build time 
failure), I thought that its disappearance after applying this patch was a 
signal thatI was doing the right thing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4056612 , @barannikov88 
wrote:

> In D141581#4056503 , @fpetrogalli 
> wrote:
>
>> This is because the sources of clangBasic and clangDriver might be compiled 
>> before LLVMTargetParser is ready.
>
> ...
>
>> Therefore, if we say that clangDriver and clangBasic depend on 
>> LLVMTargetParser we make sure that the inclusion of the tablegen-generated 
>> file resolves correctly.
>
> Sorry, I don't follow. If I read correctly, you're saying that clang 
> libraries might begin to //compile// before their DEPENDS dependency is built 
> (implying that DEPENDS clause only guarantees that the dependency is ready at 
> //link// stage). If it is true, the proposed patch changes nothing -- the 
> sources might still start to compile before cmake decides to generate inc 
> file, because it is only needed at link stage.
> Am I missing something?

Ops, yeah, I think I am wrong. In fact , when `RISCVTargetParserTableGen` is in 
the `DEPENDS`, it should be built before the clangBasic start to compile. This 
means that I actually do not know why this change fixes the issue reported 


@mgorny - can you try this patch on your workflow? Maybe the issue disappeared 
in my local machine just because for some reason it changed the order of 
compilation?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D141581#4056503 , @fpetrogalli 
wrote:

> This is because the sources of clangBasic and clangDriver might be compiled 
> before LLVMTargetParser is ready.

...

> Therefore, if we say that clangDriver and clangBasic depend on 
> LLVMTargetParser we make sure that the inclusion of the tablegen-generated 
> file resolves correctly.

Sorry, I don't follow. If I read correctly, you're saying that clang libraries 
might begin to //compile// before their DEPENDS dependency is built (implying 
that DEPENDS clause only guarantees that the dependency is ready at //link// 
stage). If it is true, the proposed patch changes nothing -- the sources might 
still start to compile before cmake decides to generate inc file, because it is 
only needed at link stage.
Am I missing something?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4056492 , @tschuett wrote:

> I though Clang Basic is a leaf library and must not depend on anything.

I wasn't aware of this requirement.  As of https://reviews.llvm.org/D137517 it 
depends on some auto-generated target information stored in `LLVMTargetParser` 
and `llvm/lib/Target/Target/RISCV/RISCV.td`. At the end, it is not even 
depending on the LLVMTargetParser library, it is just one header file in the 
public interface of the `TargetParser` that is needed (see explanation in 
https://reviews.llvm.org/D141581#4056503)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D141581#4056464 , @lebedev.ri 
wrote:

> Why is it not sufficient to link to `RISCVTargetParserTableGen`, but is 
> sufficient to link to `LLVMTargetParser`?

This is because  the sources of clangBasic and clangDriver might be compiled 
before LLVMTargetParser is ready. In this case, compilation would fail because 
both libraries include the header file `llvm/TargetParser/RISCVTargetParser.h`, 
which needs the `inc` file generated by `RISCVTargetParserTableGen`.

  // quoting code from llvm/TargetParser/RISCVTargetParser.h
  enum CPUKind : unsigned {
  #define PROC(ENUM, NAME, FEATURES, DEFAULT_MARCH) CK_##ENUM,
  #define TUNE_PROC(ENUM, NAME) CK_##ENUM,
  #include "llvm/TargetParser/RISCVTargetParserDef.inc"
  };



> Does `RISCVTargetParserTableGen` itself not link to `LLVMTargetParser`?

Nope, there is no "linking" between these two components. It is just that 
`LLVMTargetParser` requires `RISCVTargetParserTableGen`. Therefore, if we say 
that `clangDriver` and `clangBasic` depend on `LLVMTargetParser` we make sure 
that the inclusion of the tablegen-generated file resolves correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

I though Clang Basic is a leaf library and must not depend on anything.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Why is it not sufficient to link to `RISCVTargetParserTableGen`, but is 
sufficient to link to `LLVMTargetParser`?
Does `RISCVTargetParserTableGen` itself not link to `LLVMTargetParser`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

@jrtc27, @thakis, @craig.topper - gentle ping,  it would be great if I could 
unlock @mgorny with this patch for the issue they are seeing at 
https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf

Francesco


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a subscriber: mgorny.
fpetrogalli added a comment.

FWIW, the change in this patch solves the issue with standalone build of 
`clang` reported in 
https://reviews.llvm.org/rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf. (FYI, 
@mgorny )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a subscriber: craig.topper.
fpetrogalli added a comment.

@jrtc27, @thakis, @craig.topper  - is this a better solution to the issue 
raised in https://reviews.llvm.org/D137517?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141581/new/

https://reviews.llvm.org/D141581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141581: [clang] Make clangBasic and clangDriver depend on LLVMTargetParser.

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli created this revision.
fpetrogalli added a reviewer: jrtc27.
Herald added subscribers: luismarques, s.egerton, kadircet, PkmX, arphaman, 
simoncook, arichardson.
Herald added a project: All.
fpetrogalli requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay, ilya-biryukov.
Herald added a project: clang.

The header file `llvm/include/llvm/Targetparser/RISCVTargetParser.h`
relies on the auto-generated *.inc file associated to the tablegen
target `RISCVTargetParserTableGen`.

Both clangBasic and clangDriver include `RISCVTargetParser.h`,
therefore we need to make sure that the *.inc file is avaiable to
avoid compilation errors like the following:

  FAILED: 
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/RISCV.cpp.o
  /usr/bin/c++  [bunch of non interesting stuff]  -c 
/llvm-project/clang/lib/Basic/Targets/RISCV.cpp
  In file included from 
/llvm-project/clang/lib/Basic/Targets/RISCV.cpp:19:
  
/llvm-project/llvm/include/llvm/TargetParser/RISCVTargetParser.h:29:10:
 fatal error: llvm/TargetParser/RISCVTargetParserDef.inc: No such file or 
directory
29 | #include "llvm/TargetParser/RISCVTargetParserDef.inc"
   |  ^~~~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141581

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Driver/CMakeLists.txt


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -93,7 +93,7 @@
 
   DEPENDS
   ClangDriverOptions
-  RISCVTargetParserTableGen
+  LLVMTargetParser
 
   LINK_LIBS
   clangBasic
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -110,7 +110,7 @@
 
   DEPENDS
   omp_gen
-  RISCVTargetParserTableGen
+  LLVMTargetParser
   )
 
 target_link_libraries(clangBasic


Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -93,7 +93,7 @@
 
   DEPENDS
   ClangDriverOptions
-  RISCVTargetParserTableGen
+  LLVMTargetParser
 
   LINK_LIBS
   clangBasic
Index: clang/lib/Basic/CMakeLists.txt
===
--- clang/lib/Basic/CMakeLists.txt
+++ clang/lib/Basic/CMakeLists.txt
@@ -110,7 +110,7 @@
 
   DEPENDS
   omp_gen
-  RISCVTargetParserTableGen
+  LLVMTargetParser
   )
 
 target_link_libraries(clangBasic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits