mstorsjo added a comment.

In D131052#3731894 <https://reviews.llvm.org/D131052#3731894>, @sammccall wrote:

> FWIW I have no idea: in principle this makes sense, but I don't use such a 
> configuration and don't have a clear idea of what people who do use it want.

Thanks for having a look and discussing the matter! FWIW, my current cross 
compilation script does something like this: 
https://github.com/mstorsjo/llvm-mingw/blob/master/build-llvm.sh#L176-L195 All 
of those lines could be changed into setting this one option.

When there's more native tools added, I want to keep this in sync to avoid 
needless recompilation of those tools - which is kinda burdensome in the 
current setup - while with this option, we'd only need to set one option and be 
done with it. (As long as everything else works, if there's a new tool that I 
haven't got hooked up, it "only" makes things slower to build.)

> It also adds significant CMake complexity: e.g. clang-pseudo-gen now has 30 
> lines just to support the non-default "native tools" configuration, and this 
> is duplicated for each tool. Maybe this could be made cheaper by sharing more 
> of this logic in a separate place, but we should probably only add it at all 
> if this really is helping someone significantly.
>
> (Lines of CMake logic are extremely expensive to maintain IME: tooling and 
> diagnostics are poor, there are no tests, there are too many configurations 
> to reason about, interacting components are not documented, the language is 
> inherently confusing and many of us that maintain it have only a rudimentary 
> understanding)

This is a very good point indeed.

Most of the boilerplate for setting up clang-pseudo-gen and 
clang-tidy-confusable-chars-gen is identical, so those could probably be merged 
into a macro (hiding all the cross/native complexity entirely from the business 
logic, just like for tablegen) - but the complexity is still there. (It would 
end up in at least three cases; tablegen, llvm-config and in a 
generic-buildtime-tool-macro.)

On the topic of "really is helping someone significantly" - if we wouldn't have 
the burden of existing users using the existing options (for setting the path 
to each tool individually), picking this new option is a nobrainer - doing the 
same thing but in a much less annoying way. But since we have existing users 
with the existing options (which would need to be kept for some time 
transitionally), it's much less clear cut whether it's a "significant" 
improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

Reply via email to