[PATCH] D70306: clang: Exherbo multiarch ajustments
Keruspe planned changes to this revision. Keruspe marked 3 inline comments as done. Keruspe added a comment. Will give llvm::sys::path::stem a look and try to drop the x86_64 part from the include dir selection Comment at: clang/lib/Driver/ToolChains/Linux.cpp:360 + addPathIfExists(D, + LibPath + "/../../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), compnerd wrote: > Could you use `llvm::sys::path::stem` please? This was just a copy of the code that as already there, with a different path, but I sure can give that a look Comment at: clang/lib/Driver/ToolChains/Linux.cpp:889 + break; +} + } compnerd wrote: > Why not just always construct the path? `"/usr/" + getTriple.str() + > "/include"` is always going to be the path that we have for exherbo That's what we had at first, but then e.g. `clang -m32` didn't work as it was searching inside of i383 and not i686, and wrt x86_64 it was using unknown at some point instead of pc. The x86_64 part might have been because it was in another environment but the x86 part definitely is still relevant Comment at: clang/lib/Driver/ToolChains/Linux.cpp:984 + if (Distro == Distro::Exherbo && + addLibStdCXXIncludePaths( + LibDir.str() + "/../../" + TripleStr + "/include", compnerd wrote: > Why is this part of the `if`? This should be part of the code executed > conditionally. It just matches the if from below, with the distro check added. Not familiar with the llvm codebase so I'm trying to just stick to the similar code that's just next to it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70306/new/ https://reviews.llvm.org/D70306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70306: clang: Exherbo multiarch ajustments
compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. The test adjustments are incorrect. They should optionally accept the triple in the path. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:360 + addPathIfExists(D, + LibPath + "/../../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), Could you use `llvm::sys::path::stem` please? Comment at: clang/lib/Driver/ToolChains/Linux.cpp:889 + break; +} + } Why not just always construct the path? `"/usr/" + getTriple.str() + "/include"` is always going to be the path that we have for exherbo Comment at: clang/lib/Driver/ToolChains/Linux.cpp:984 + if (Distro == Distro::Exherbo && + addLibStdCXXIncludePaths( + LibDir.str() + "/../../" + TripleStr + "/include", Why is this part of the `if`? This should be part of the code executed conditionally. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70306/new/ https://reviews.llvm.org/D70306 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70306: clang: Exherbo multiarch ajustments
Keruspe updated this revision to Diff 229682. Keruspe added a comment. Herald added subscribers: ormris, atanasyan. Mark some tests as XFAIL on exherbo because the filesystem layout is different CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70306/new/ https://reviews.llvm.org/D70306 Files: clang/lib/Driver/ToolChains/Linux.cpp clang/test/Driver/android-ndk-standalone.cpp clang/test/Driver/arm-multilibs.c clang/test/Driver/cross-linux.c clang/test/Driver/dyld-prefix.c clang/test/Driver/linux-header-search.cpp clang/test/Driver/linux-ld.c clang/test/Driver/mips-cs.cpp clang/test/Driver/mips-fsf.cpp clang/test/Driver/mips-img.cpp clang/test/Headers/arm-fp16-header.c clang/test/Headers/arm-neon-header.c clang/test/lit.cfg.py Index: clang/test/lit.cfg.py === --- clang/test/lit.cfg.py +++ clang/test/lit.cfg.py @@ -193,3 +193,6 @@ if config.enable_shared: config.available_features.add("enable_shared") + +if os.path.exists('/etc/exherbo-release'): +config.available_features.add('exherbo') Index: clang/test/Headers/arm-neon-header.c === --- clang/test/Headers/arm-neon-header.c +++ clang/test/Headers/arm-neon-header.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -flax-vector-conversions=none -ffreestanding %s // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s Index: clang/test/Headers/arm-fp16-header.c === --- clang/test/Headers/arm-fp16-header.c +++ clang/test/Headers/arm-fp16-header.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // RUN: %clang -fsyntax-only -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s Index: clang/test/Driver/mips-img.cpp === --- clang/test/Driver/mips-img.cpp +++ clang/test/Driver/mips-img.cpp @@ -1,4 +1,5 @@ // REQUIRES: mips-registered-target +// XFAIL: exherbo // Check frontend and linker invocations on the IMG MIPS toolchain. // Index: clang/test/Driver/mips-fsf.cpp === --- clang/test/Driver/mips-fsf.cpp +++ clang/test/Driver/mips-fsf.cpp @@ -1,4 +1,5 @@ // REQUIRES: mips-registered-target +// XFAIL: exherbo // Check frontend and linker invocations on FSF MIPS toolchain. // Index: clang/test/Driver/mips-cs.cpp === --- clang/test/Driver/mips-cs.cpp +++ clang/test/Driver/mips-cs.cpp @@ -1,4 +1,5 @@ // REQUIRES: mips-registered-target +// XFAIL: exherbo // // Check frontend and linker invocations on Mentor Graphics MIPS toolchain. // Index: clang/test/Driver/linux-ld.c === --- clang/test/Driver/linux-ld.c +++ clang/test/Driver/linux-ld.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // General tests that ld invocations on Linux targets sane. Note that we use // sysroot to make these tests independent of the host system. // Index: clang/test/Driver/linux-header-search.cpp === --- clang/test/Driver/linux-header-search.cpp +++ clang/test/Driver/linux-header-search.cpp @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // General tests that the header search paths detected by the driver and passed // to CC1 are sane. // Index: clang/test/Driver/dyld-prefix.c === --- clang/test/Driver/dyld-prefix.c +++ clang/test/Driver/dyld-prefix.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // RUN: touch %t.o // RUN: %clang -target i386-unknown-linux --dyld-prefix /foo -### %t.o 2>&1 | FileCheck --check-prefix=CHECK-32 %s Index: clang/test/Driver/cross-linux.c === --- clang/test/Driver/cross-linux.c +++ clang/test/Driver/cross-linux.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // RUN: %clang -### -o %t %s 2>&1 -no-integrated-as -fuse-ld=ld \ // RUN: --gcc-toolchain=%S/Inputs/basic_cross_linux_tree/usr \ // RUN: --target=i386-unknown-linux-gnu \ Index: clang/test/Driver/arm-multilibs.c === --- clang/test/Driver/arm-multilibs.c +++ clang/test/Driver/arm-multilibs.c @@ -1,3 +1,5 @@ +// XFAIL: exherbo + // RUN: %cl
[PATCH] D70306: clang: Exherbo multiarch ajustments
Keruspe updated this revision to Diff 229600. Keruspe added a comment. Make Exherbo multiarch integration more similar to the debian one Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70306/new/ https://reviews.llvm.org/D70306 Files: clang/lib/Driver/ToolChains/Linux.cpp Index: clang/lib/Driver/ToolChains/Linux.cpp === --- clang/lib/Driver/ToolChains/Linux.cpp +++ clang/lib/Driver/ToolChains/Linux.cpp @@ -350,9 +350,21 @@ // // Note that this matches the GCC behavior. See the below comment for where // Clang diverges from GCC's behavior. -addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" + - OSLibDir + SelectedMultilib.osSuffix(), -Paths); +// +// On Exherbo, the GCC installation will reside in e.g. +// /usr/x86_64-pc-linux-gnu/lib/gcc/armv7-unknown-linux-gnueabihf/9.2.0 +// while the matching lib path is +// /usr/armv7-unknown-linux-gnueabihf/lib +if (Distro == Distro::Exherbo) + addPathIfExists(D, + LibPath + "/../../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), + Paths); +else + addPathIfExists(D, + LibPath + "/../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), + Paths); // If the GCC installation we found is inside of the sysroot, we want to // prefer libraries installed in the parent prefix of the GCC installation. @@ -844,6 +856,39 @@ if (getTriple().isAndroid()) MultiarchIncludeDirs = AndroidMultiarchIncludeDirs; + // Exherbo's multiarch layout is /usr//include and not + // /usr/include/ + const Distro Distro(D.getVFS()); + const StringRef ExherboMultiarchIncludeDirs[] = {"/usr/" + getTriple().str() + + "/include"}; + const StringRef ExherboX86_64MuslMultiarchIncludeDirs[] = { + "/usr/x86_64-pc-linux-musl/include"}; + const StringRef ExherboX86_64MultiarchIncludeDirs[] = { + "/usr/x86_64-pc-linux-gnu/include"}; + const StringRef ExherboI686MuslMultiarchIncludeDirs[] = { + "/usr/i686-pc-linux-musl/include"}; + const StringRef ExherboI686MultiarchIncludeDirs[] = { + "/usr/i686-pc-linux-gnu/include"}; + if (Distro == Distro::Exherbo) { +switch (getTriple().getArch()) { +case llvm::Triple::x86_64: + if (getTriple().isMusl()) +MultiarchIncludeDirs = ExherboX86_64MuslMultiarchIncludeDirs; + else +MultiarchIncludeDirs = ExherboX86_64MultiarchIncludeDirs; + break; +case llvm::Triple::x86: + if (getTriple().isMusl()) +MultiarchIncludeDirs = ExherboI686MuslMultiarchIncludeDirs; + else +MultiarchIncludeDirs = ExherboI686MultiarchIncludeDirs; + break; +default: + MultiarchIncludeDirs = ExherboMultiarchIncludeDirs; + break; +} + } + for (StringRef Dir : MultiarchIncludeDirs) { if (D.getVFS().exists(SysRoot + Dir)) { addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + Dir); @@ -917,12 +962,30 @@ StringRef LibDir = GCCInstallation.getParentLibPath(); StringRef InstallDir = GCCInstallation.getInstallPath(); StringRef TripleStr = GCCInstallation.getTriple().str(); + const Driver &D = getDriver(); const Multilib &Multilib = GCCInstallation.getMultilib(); - const std::string GCCMultiarchTriple = getMultiarchTriple( - getDriver(), GCCInstallation.getTriple(), getDriver().SysRoot); + const std::string GCCMultiarchTriple = + getMultiarchTriple(D, GCCInstallation.getTriple(), D.SysRoot); const std::string TargetMultiarchTriple = - getMultiarchTriple(getDriver(), getTriple(), getDriver().SysRoot); + getMultiarchTriple(D, getTriple(), D.SysRoot); const GCCVersion &Version = GCCInstallation.getVersion(); + const Distro Distro(D.getVFS()); + + // On Exherbo the consecutive addLibStdCXXIncludePaths call would evaluate to: + // LibDir = /usr/lib/gcc//9.2.0/../../.. + // = /usr/lib/ + // LibDir + "/../include" = /usr/include + // addLibStdCXXIncludePaths would then check if "/usr/include/c++/" + // exists, and add that as include path when what we want is + // "/usr//include/c++/" + // Note that "/../../" is needed and not just "/../" as /usr/include points to + // /usr/host/include + if (Distro == Distro::Exherbo && + addLibStdCXXIncludePaths( + LibDir.str() + "/../../" + TripleStr + "/include", + "/c++/" + Version.Text, TripleStr, GCCMultiarchTriple, + TargetMultiarchTriple, Multilib.includeSuffix(), DriverArgs, CC1Args)) +return; // The primary search for libstdc++ supports multiarch variants. if (addLibStdCXXIncludePaths(LibDir.str() + "/../include",
[PATCH] D70306: clang: Exherbo multiarch ajustments
Keruspe created this revision. Keruspe added reviewers: dlj, rsmith, compnerd. Keruspe added a project: clang. Herald added a subscriber: cfe-commits. Exherbo multiarch layout being somewhat specific, some adjustments need to be made wrt the lookup paths. Based in parts on patches by Marvin Schmidt Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70306 Files: clang/lib/Driver/ToolChains/Linux.cpp Index: clang/lib/Driver/ToolChains/Linux.cpp === --- clang/lib/Driver/ToolChains/Linux.cpp +++ clang/lib/Driver/ToolChains/Linux.cpp @@ -350,9 +350,21 @@ // // Note that this matches the GCC behavior. See the below comment for where // Clang diverges from GCC's behavior. -addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" + - OSLibDir + SelectedMultilib.osSuffix(), -Paths); +// +// On Exherbo, the GCC installation will reside in e.g. +// /usr/x86_64-pc-linux-gnu/lib/gcc/armv7-unknown-linux-gnueabihf/9.2.0 +// while the matching lib path is +// /usr/armv7-unknown-linux-gnueabihf/lib +if (Distro == Distro::Exherbo) + addPathIfExists(D, + LibPath + "/../../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), + Paths); +else + addPathIfExists(D, + LibPath + "/../" + GCCTriple.str() + "/lib/../" + + OSLibDir + SelectedMultilib.osSuffix(), + Paths); // If the GCC installation we found is inside of the sysroot, we want to // prefer libraries installed in the parent prefix of the GCC installation. @@ -844,6 +856,31 @@ if (getTriple().isAndroid()) MultiarchIncludeDirs = AndroidMultiarchIncludeDirs; + // Exherbo's multiarch layout is /usr//include and not + // /usr/include/ + const Distro Distro(D.getVFS()); + std::string ExherboMultiarchIncludeDir = + std::string("/usr/") + getTriple().str() + "/include"; + if (Distro == Distro::Exherbo) { +switch (getTriple().getArch()) { +case llvm::Triple::x86_64: + if (getTriple().isMusl()) +ExherboMultiarchIncludeDir = "/usr/x86_64-pc-linux-musl/include"; + else +ExherboMultiarchIncludeDir = "/usr/x86_64-pc-linux-gnu/include"; + break; +case llvm::Triple::x86: + if (getTriple().isMusl()) +ExherboMultiarchIncludeDir = "/usr/i686-pc-linux-musl/include"; + else +ExherboMultiarchIncludeDir = "/usr/i686-pc-linux-gnu/include"; + break; +default: + break; +} +MultiarchIncludeDirs = {ExherboMultiarchIncludeDir}; + } + for (StringRef Dir : MultiarchIncludeDirs) { if (D.getVFS().exists(SysRoot + Dir)) { addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + Dir); @@ -917,12 +954,30 @@ StringRef LibDir = GCCInstallation.getParentLibPath(); StringRef InstallDir = GCCInstallation.getInstallPath(); StringRef TripleStr = GCCInstallation.getTriple().str(); + const Driver &D = getDriver(); const Multilib &Multilib = GCCInstallation.getMultilib(); - const std::string GCCMultiarchTriple = getMultiarchTriple( - getDriver(), GCCInstallation.getTriple(), getDriver().SysRoot); + const std::string GCCMultiarchTriple = + getMultiarchTriple(D, GCCInstallation.getTriple(), D.SysRoot); const std::string TargetMultiarchTriple = - getMultiarchTriple(getDriver(), getTriple(), getDriver().SysRoot); + getMultiarchTriple(D, getTriple(), D.SysRoot); const GCCVersion &Version = GCCInstallation.getVersion(); + const Distro Distro(D.getVFS()); + + // On Exherbo the consecutive addLibStdCXXIncludePaths call would evaluate to: + // LibDir = /usr/lib/gcc//9.2.0/../../.. + // = /usr/lib/ + // LibDir + "/../include" = /usr/include + // addLibStdCXXIncludePaths would then check if "/usr/include/c++/" + // exists, and add that as include path when what we want is + // "/usr//include/c++/" + // Note that "/../../" is needed and not just "/../" as /usr/include points to + // /usr/host/include + if (Distro == Distro::Exherbo && + addLibStdCXXIncludePaths( + LibDir.str() + "/../../" + TripleStr + "/include", + "/c++/" + Version.Text, TripleStr, GCCMultiarchTriple, + TargetMultiarchTriple, Multilib.includeSuffix(), DriverArgs, CC1Args)) +return; // The primary search for libstdc++ supports multiarch variants. if (addLibStdCXXIncludePaths(LibDir.str() + "/../include", Index: clang/lib/Driver/ToolChains/Linux.cpp === --- clang/lib/Driver/ToolChains/Linux.cpp +++ clang/lib/Driver/ToolChains/Linux.cpp @@ -350,9 +350,21 @@ // // Note that this matches the GCC behavior. See the below comment for where // Clang diverges from GCC's behavior. -addPa