rnk added inline comments.
================
Comment at: clang/lib/Driver/ToolChain.cpp:446
case ToolChain::FT_Shared:
- Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+ Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
----------------
This is the same as `TT`, right? Please use TT here or remove TT and use Triple
across this function. I guess I lean towards keeping TT, but it's up to you.
================
Comment at: clang/lib/Driver/ToolChain.cpp:468-469
// Check for runtime files in the new layout without the architecture first.
- std::string CRTBasename =
- buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
- for (const auto &LibPath : getLibraryPaths()) {
- SmallString<128> P(LibPath);
- llvm::sys::path::append(P, CRTBasename);
- if (getVFS().exists(P))
- return std::string(P.str());
+ std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+ if (auto value = findInLibraryPath(*this, CRTBasename)) {
+ return value.getValue();
----------------
This does twice as many stat syscalls as we need. That seems worth optimizing.
First, in getCompilerRTBasename, we search the library path, we return a
result, and then we search it again here. I think the nicest solution is
probably to have one shared implementation that takes a boolean and returns the
full path or the basename.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96638/new/
https://reviews.llvm.org/D96638
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
