[PATCH] D110900: Triple: Add RedHat vendor

Harald van Dijk via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 3 20:58:50 PDT 2023


hvdijk added a comment.

In D110900#4242187 <https://reviews.llvm.org/D110900#4242187>, @MaskRay wrote:

> Can you elaborate? I agree that decreasing the number of filesystem stats is preferable...

My thinking there was that, taking `x86_64-redhat-linux-gnu` as an example, we know that the GCC installation we are looking for is going to use a target name of exactly `x86_64-redhat-linux`, so we can search for only that and skip the search for other target names, and skip the file system accesses associated with the search for other target names. And if we do it that way, there is no longer a drawback to using `x86_64-redhat-linux-gnu` as the LLVM triple, so we can simply use that and avoid the special vendor-based exceptions in `isGNUEnvironment()` that we have in this diff.

But, I don't actually mind doing it like in this diff; please consider it only as an option, and if you don't like it, just stick with the perfectly good diff we have here already.



================
Comment at: llvm/include/llvm/TargetParser/Triple.h:579
+    case Triple::UnknownEnvironment:
+        return getVendor() == RedHat;
+    default:
----------------
MaskRay wrote:
> tstellar wrote:
> > aaronpuchert wrote:
> > > hvdijk wrote:
> > > > In D110900#4160169 you suggested that this should be done for SUSE as well, do you still think so?
> > > Yes, in fact we ship with a [patch](https://build.opensuse.org/package/view_file/devel:tools:compiler/llvm16/llvm-suse-implicit-gnu.patch) that does that in openSUSE now. It mostly works without that, but some things were missing, see D110900#4160110.
> > > 
> > > If we can agree on this, I'd open a separate change on top of this for `SUSE`. (Unless @tstellar would like to include it right away.)
> > I think we should add SUSE here in a separate change.
> > In D110900#4160169 you suggested that this should be done for SUSE as well, do you still think so?
> 
> @hvdijk Your suggestion may be this: "
> A possibly radical alternative suggestion: could we instead change the way the GCC installation to use is determined"
> 
> Can you elaborate? I agree that decreasing the number of filesystem stats is preferable...
This is unrelated to what is being discussed here, will respond in a non-inline comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110900



More information about the llvm-commits mailing list