Cuda and rocm toolchain detectors are currently run unconditionally, while their result may not be used at all. Make their initialization lazy so that the discovery code is not run in common cases.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Does this mean that clang will no longer search for the ROCM and CUDA library paths for every C compile?
That's great! Thanks for working on this. I think this looks OK, but someone with more C++ knowledge than me should probably look over the LazyDetector class implementation.
clang/lib/Driver/ToolChains/LazyDetector.h | ||
---|---|---|
27 | Any reason this is mutable? I don't see it being used a non-const way in a const method |
clang/lib/Driver/ToolChains/LazyDetector.h | ||
---|---|---|
27 | yeah, T const *operator->() const updates the optional by actually creating the value. (std::optional::empalce is not const) |
clang/lib/Driver/ToolChains/LazyDetector.h | ||
---|---|---|
27 | Your T const *operator->() const method does not do so, T *operator->() does, which is non-const and hence can call emplace without issues. |
clang/lib/Driver/ToolChains/LazyDetector.h | ||
---|---|---|
37 | Is the .value() here permitted? AFAIK the convention is to always use operator*. And I think @aaron.ballman would tell you to drop the {} around that single-statement if :) |
LGTM
clang/lib/Driver/ToolChains/LazyDetector.h | ||
---|---|---|
37 | llvm style prefers const T to T const |
Hello all,
sorry to be a bother, but I believe this patch has caused a failure in this buildbot (and potentially others).
This broke clang/test/Driver/rocm-detect.hip on a number of platforms (including the pre-merge checks on this PR), I've reverted for now in b5ee4f755fcff56243f6ff0cea9e7a722259304a.
(The first line of 0ffaffcaac97de31e1b0e7e80c4f7cab724eda20 should have mentioned Lazyly initialize uncommon toolchain detector. You can add Reland prefix or add the message in the body of the commit message.)
I fixed a -fsanitize-address-stack-use-after-scope issue in 6ab9f1e59371fe96ca3fda1a26a28ae0b7caf637
Any reason this is mutable? I don't see it being used a non-const way in a const method