I have updated the patch modifying the SubtargetFeature definition instead of the Predicate NAME.

In D146129#4196146, @asb wrote:

Thanks for making this suggestion.

The suggested renaming would be consistent with those other predicates, but RV{32,64}{I,E} are different base ISAs while the other predicates refer to instruction set extensions. Because of this logical distinction IsRV32/IsRV64 make more sense to me at least.

It's a bit less clear for RVE given the way me model it within LLVM is somewhat like as if it was an instruction set extension. Though I still think I personally prefer IsRVE for that case.

LGTM, with a minor comments on the docs, to make somehow explicit that these new operators accept only string in input.

In D145300#4170807, @zjaffal wrote:

In D145300#4170805, @fpetrogalli wrote:

Thank you @zjaffal for working on this.

Please update also the docs at https://llvm.org/docs/TableGen/ProgRef.html

Wouldn't that page be updated if I update TableGen/ProgRef.rst ?

Thank you @zjaffal for working on this.

Self approving a non-controversial trivial change.

Remove empty line from unrelated code. NFC

1. Added continue if (!SU).
2. Removed code I accidentally uploaded that was making pre-commit unhappy: https://reviews.llvm.org/harbormaster/unit/209975/ (well, OK this is an intended functional change... ;) )

Address code review. NFC

In D142403#4075215, @tstellar wrote:

LGTM. I tested and this fixes the build.

In D142403#4075215, @tstellar wrote:

LGTM. I tested and this fixes the build.

Address code review.

In D142403#4074885, @tstellar wrote:

This is still the wrong change IMO. I don't know, maybe I'm not being clear, but I don't 'think you ever actually tried my suggestion in D141581 which was to leave the RISCVTargetParserTableGen Depends as is and add the pseudo targets in llvm/cmake/modules/LLVMConfig.cmake.in.

This was essentially reverted in https://reviews.llvm.org/rG20ffb9aaf982, so I am abandoning it in favour of https://reviews.llvm.org/D142403

Reverted in https://reviews.llvm.org/rG20ffb9aaf982

Ah - there you go. Without a proper chained dependency of LLVMTargetParser -> clangBasic we end up with failures if clangBasic is compiled before LLVMTargetParser is (even on non stand-alone builds). https://lab.llvm.org/buildbot/#/builders/193/builds/25362 :

In D141581#4074312, @tstellar wrote:

I tested this patch and it works, so I'll give it an LGTM. However, I think the LLVMConfig.cmake.in changes are now unnecessary since the RISCVTargetParserTableGen has been dropped from all clang targets.

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

LGTM, thanks.

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?

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?

In D141581#4069053, @tstellar wrote:

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

In D141650#4064127, @Nuullll wrote:

In D141650#4056383, @lebedev.ri wrote:

In D141650#4055313, @Nuullll wrote:

Thanks a lot for reviewing!

@lebedev.ri

1. Do we need that parameter? Is empty <parameters> list generally ill-formed and must be diagnosed?

There's one subtle difference in AArch64 and X86 VFABI documentations about Vector Function Name Mangling: the X86 doc explicitly allows an empty parameter list while the AArch64 doc doesn't -- so I think this is why we need a parameter to let users choose whatever standard to conform. In fact, our internal implementation has to remove the diagnostic on empty parameter from tryDemangleForVFABI to make sure we're conforming the X86 VFABI.

Nitpick: would it be possible to simply ask AArch64 for clarification, what is the intended behaviour for them?

@fpetrogalli Can you please kindly provide some inputs on AArch64 VFABI? Reference commit: https://github.com/ARM-software/abi-aa/commit/8fcf3f72d7cc200c50a4b029cee295469ab46fe5

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

Update the example in the test to have non-zero values printed for both TopReadyCycle and BottomReadyCycle.

Based on offline feedback from @fhahn, I have:

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?

@barannikov88 - I am stuck with an incomplete explanation:

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?

In D141581#4056492, @tschuett wrote:

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

In D141581#4056464, @lebedev.ri wrote:

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

@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

Thank you @tstellar

s/standalone/stand-alone/

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

In rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf#1164455, @mgorny wrote:

Ping. Please either fix this or revert and put the new version back for review.

In rGac1ffd3caca12c254e0b8c847aa8ce8e51b6cfbf#1164086, @mgorny wrote:

This version breaks standalone builds of clang:

CMake Error at /usr/lib/llvm/16/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)

Specify -mtriple in the invocation of llc in the test to prevent it fail on targets that do not support the cpu specified.

Rebase. NFC. (Sorting my head on how properly do a recommit, apologies for the noise).

Add -mtriple to the invocation of llc in the test to prevent it fail on non-aarch64 machines.

Reopening to update with a rework after a revert of the original submission to main

Rebase on main branch. NFC.