[PATCH] D142688: [Clang][Driver] Handle LoongArch multiarch tuples

[WÁNG Xuěruì via Phabricator via cfe-commits]

xen0n added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:41
 /// so we provide a rough mapping here.
 std::string Linux::getMultiarchTriple(const Driver &D,
                                       const llvm::Triple &TargetTriple,
----------------
SixWeining wrote:
> Missing test. Perhaps add some in `clang/test/Driver/linux-ld.c` and `clang/test/Driver/linux-header-search.cpp`? Or postpone this change until loongarch is upstreamed to Debian?
Thanks, I'll try adding the tests in the next revision.

But given [[ https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028654 | the recent reversal of upstream dpkg support ]] (apparently due to miscommunication) it may be prudent to wait until a consensus is reached Debian-side.


================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:93
+
+    if (TargetTriple.isGNUEnvironment()) {
+      Libc = "gnu";
----------------
SixWeining wrote:
> Should only check `{GNU|GNUF32|GNUF64}`?
While technically only `gnu{,sf,f32,f64}` are permitted by the LoongArch Toolchain Conventions (with the bare `-gnu` being somewhat de-facto standard but not officially so), I believe the user's intent to specify a GNU environment still holds even in cases of "errors" like applying "gnuabi64" or "gnueabi" to LoongArch. Same applies for the musl case.


================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:95
+      Libc = "gnu";
+    } else if (TargetTriple.isMusl()) {
+      Libc = "musl";
----------------
SixWeining wrote:
> Should only check `Triple::Musl`?
Explained above in the GNU case.


================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:116
+
+    return std::string("loongarch64-linux-") + Libc + FPFlavor;
+  }
----------------
MaskRay wrote:
> `return (... + ...).str();` if an operand of `+` is a `Twine`
Thanks, will change in next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688