[PATCH] D112890: headers: optionalise some generated resource headers

thakis added a comment. (…at least for the arm headers, which 1. didn't use to do this 2. are a lot smaller than the risc-v headers ) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890

[PATCH] D112890: headers: optionalise some generated resource headers

thakis added a comment. In D112890#3119897 , @jrtc27 wrote: > This change seems pretty counter to Clang's ability to spit out IR for > anything regardless of what backends you have. The right fix should be to > shrink the headers, not mask the problem b

[PATCH] D112890: headers: optionalise some generated resource headers

jrtc27 added a comment. And I don't see any AArch64 or ARM maintainers ACKing this. You haven't even tagged any as reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 _

[PATCH] D112890: headers: optionalise some generated resource headers

jrtc27 added a comment. This change seems pretty counter to Clang's ability to spit out IR for anything regardless of what backends you have. The right fix should be to shrink the headers, not mask the problem by changing the principles of how Clang works. Repository: rG LLVM Github Monorepo

[PATCH] D112890: headers: optionalise some generated resource headers

MaskRay added a comment. But thanks for the work: people not enabling RISCV should not incur so much overhead. I hope that RISC-V vector contributors can really bear the cost in mind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ http

[PATCH] D112890: headers: optionalise some generated resource headers

MaskRay added a comment. I remember that `NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py` needs to be the first line... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 ___

[PATCH] D112890: headers: optionalise some generated resource headers

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 ___

[PATCH] D112890: headers: optionalise some generated resource headers

compnerd added a comment. @craig.topper - ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D112890: headers: optionalise some generated resource headers

rnk added a comment. I think this seems reasonable. Generally speaking, we have tests in clang that generate IR for targets that are not enabled in LLVM. It is kind of nice: you don't have to mark the IRGen tests with `REQUIRES: foo-registered-target`. If we want to test these ARM and RISCV in

[PATCH] D112890: headers: optionalise some generated resource headers

compnerd added a comment. In D112890#3099256 , @tschuett wrote: > If I understand you correctly, I would need to pass something ala `-target > riscv-xx` to enable `__riscv_vector`. However, this is impossible because > the risk target is disabled. So I

[PATCH] D112890: headers: optionalise some generated resource headers

tschuett added a comment. In D112890#3099258 , @etcwilde wrote: > Right, the header is only necessary when you're targeting riscv, so if > compiling for riscv isn't supported by the compiler noted in the > `LLVM_TARGETS_TO_BUILD`, you don't need it and

[PATCH] D112890: headers: optionalise some generated resource headers

etcwilde added a comment. Right, the header is only necessary when you're targeting riscv, so if compiling for riscv isn't supported by the compiler noted in the `LLVM_TARGETS_TO_BUILD`, you don't need it and can't use it anyway. This is separate from the host platform that the compiler is runni

[PATCH] D112890: headers: optionalise some generated resource headers

tschuett added a comment. If I understand you correctly, I would need to pass something ala `-target riscv-xx` to enable `__riscv_vector`. However, this is impossible because the risk target is disabled. So I thing it is save what you are doing. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D112890: headers: optionalise some generated resource headers

compnerd added a comment. In D112890#3099208 , @tschuett wrote: > I am undecided between this may break something and I cannot use the risc-v > headers on an x86 machine. This fundamentally cannot break something outside of the RISC-V //target//, it is

[PATCH] D112890: headers: optionalise some generated resource headers

tschuett added a comment. I am undecided between this may break something and I cannot use the risc-v headers on an x86 machine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112890/new/ https://reviews.llvm.org/D112890 __

[PATCH] D112890: headers: optionalise some generated resource headers

etcwilde added a comment. This makes sense to me. It doesn't make sense to spend the time generating files that will never get used, nor spend the space for storing them. `riscv_vector.h` in particular is quite large, coming in at just under 10M, and there are two of them showing up in my build

[PATCH] D112890: headers: optionalise some generated resource headers

compnerd created this revision. compnerd added a reviewer: craig.topper. Herald added subscribers: luke957, luismarques, s.egerton, PkmX, simoncook, kristof.beyls, mgorny. compnerd requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This split