[PATCH] D97993: [Driver] Suppress GCC detection under -B for non-Android

[Fangrui Song via Phabricator via cfe-commits]

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