[PATCH] D87187: [Driver] Perform Linux distribution detection just once

Alexandre Ganea via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 17 05:54:49 PDT 2020


aganea added a subscriber: rnk.
aganea added a comment.

Please install & run `git clang-format` before uploading the patch.



================
Comment at: clang/include/clang/Driver/Distro.h:27
+    // Special value means that no detection was performed yet.
+    UninitializedDistro = -1,
     // NB: Releases of a particular Linux distro should be kept together
----------------
You can leave out the `-1` since this isn't serialized.


================
Comment at: clang/lib/Driver/Distro.cpp:29
 
-  // If the host is not running Linux, and we're backed by a real file system,
-  // no need to check the distro. This is the case where someone is
-  // cross-compiling from BSD or Windows to Linux, and it would be meaningless
-  // to try to figure out the "distro" of the non-Linux host.
-  IntrusiveRefCntPtr<llvm::vfs::FileSystem> RealFS =
-      llvm::vfs::getRealFileSystem();
-  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
-  if (!HostTriple.isOSLinux() && &VFS == RealFS.get())
-    return Distro::UnknownDistro;
+  if (File) {
+    SmallVector<StringRef, 16> Lines;
----------------
Early exit & move L23 to L32.


================
Comment at: clang/lib/Driver/Distro.cpp:54
+
   if (File) {
     SmallVector<StringRef, 16> Lines;
----------------
Early exit to avoid indent and ease reading:
```
if (!File)
  return Distro::UnknownDistro;
```
Also move L50 to L57.


================
Comment at: clang/lib/Driver/Distro.cpp:205
+                                    const llvm::Triple &TargetOrHost) {
+  static Distro::DistroType Type = Distro::UninitializedDistro;
+
----------------
I'm not sure if this is safe. @rnk Do we expect to call `GetDistro` with a different `VFS` or a different `Triple` when compiling with Clang? For GPU targets? Shouldn't we do this higher up, on the call site?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87187/new/

https://reviews.llvm.org/D87187



More information about the cfe-commits mailing list