MaskRay added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1911 + SmallVector<std::string, 8> Prefixes; + if (TargetTriple.isAndroid()) + Prefixes.assign(D.PrefixDirs.begin(), D.PrefixDirs.end()); ---------------- srhines wrote: > danalbert wrote: > > I'm not entirely sure what `D.PrefixDirs` represents so maybe Android > > doesn't need this either. > > > > The behavior the NDK depends on is being able to find tools co-located with > > the Clang driver location. Aside from `as`, these are all LLVM tools (lld > > and co). > > > > The sysroot is expected to be in `$CLANG/../sysroot`. All our headers, > > libraries (aside from libgcc/libatomic), and CRT objects are located there. > > > > The clang driver install location is expected to also be a GCC install > > directory, so libgcc/libatomic are expected at > > `$CLANG/../lib/gcc/$TRIPLE/$GCC_VERSION`. > > > > Typical usage for the NDK does not involve `-gcc-toolchain` or `-B` at all. > > > > If I've understood correctly, your change can be applied to Android as well > > without breaking any of those behaviors. @srhines will need to comment on > > whether the Android platform build needs this, but aiui anyone depending on > > this behavior just needs to fix their build to use `-gcc-toolchain` where > > they were previously using `-B`. > > > > Of course, I can't speak to what our users with custom build systems that > > don't follow our docs might be doing. > From a look at Android's build system, we are using `-B` but we don't > currently use `--sysroot` or `-gcc-toolchain` for platform builds. I did try > a build with this patch (but not retaining the Android-specific part), and it > still worked fine. From looking at the constructed command lines, the > platform build generally adds the appropriate include paths, library paths, > and invokes separate tools directly, so we aren't relying on `-B` for this > purpose. Cleaning this up is on my list of things to eventually do, but as I > see it, we're not going to be negatively impacted even with the more > aggressive version of this patch. > > > Of course, I can't speak to what our users with custom build systems that > > don't follow our docs might be doing. > I do share this concern a bit, but it's very hard for me to quantify how hard > it would be for someone to just adapt their build if we did make a more > aggressive change here. If it's really as easy as passing `-gcc-toolchain`, > then perhaps that would be fine for the Release Notes. It should be as easy as passing `--gcc-toolchain`. I edited the summary to state more clearly that the current behavior actually makes interop with `--gcc-toolchain` difficult. I'll not touch Android to restrict the influence to regular Linux distributions in this patch. Android can be fixed separately (which require two test changes.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97993/new/ https://reviews.llvm.org/D97993 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
