Add clang driver changes to support MTI RISC-V #134065
Conversation
llvmbot
added
clang
djtodoro
force-pushed
the
pr/mti-riscv-driver
branch
from
05f04dd to
44fd7ad
Compare
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang-driver Author: Djordje Todorovic (djtodoro) ChangesAdd support for MipsTechnologies for RISC-V targets. Full diff: https://github.com/llvm/llvm-project/pull/134065.diff 6 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 1c5b5ff5e5b40..679387240aa3b 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -337,12 +337,14 @@ std::string riscv::getRISCVArch(const llvm::opt::ArgList &Args,
// - On `riscv{XLEN}-unknown-elf` we default to `rv{XLEN}imac`
// - On all other OSs we use `rv{XLEN}imafdc` (equivalent to `rv{XLEN}gc`)
if (Triple.isRISCV32()) {
- if (Triple.getOS() == llvm::Triple::UnknownOS)
+ if (Triple.getOS() == llvm::Triple::UnknownOS &&
+ Triple.getVendor() != llvm::Triple::MipsTechnologies)
return "rv32imac";
return "rv32imafdc";
}
- if (Triple.getOS() == llvm::Triple::UnknownOS)
+ if (Triple.getOS() == llvm::Triple::UnknownOS &&
+ Triple.getVendor() != llvm::Triple::MipsTechnologies)
return "rv64imac";
if (Triple.isAndroid())
return "rv64imafdcv_zba_zbb_zbs";
@@ -365,5 +367,9 @@ std::string riscv::getRISCVTargetCPU(const llvm::opt::ArgList &Args,
if (!CPU.empty())
return CPU;
+ if (Triple.getVendor() == llvm::Triple::MipsTechnologies &&
+ Triple.isRISCV64())
+ return "p8700";
+
return Triple.isRISCV64() ? "generic-rv64" : "generic-rv32";
}
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index a0fa3c66d7dec..e85f014bfb90d 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -402,6 +402,11 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(Arch == llvm::Triple::aarch64_be ? "-EB" : "-EL");
}
+ if (Triple.isRISCV() &&
+ Triple.getVendor() == llvm::Triple::MipsTechnologies) {
+ CmdArgs.push_back("-EL");
+ }
+
// Most Android ARM64 targets should enable the linker fix for erratum
// 843419. Only non-Cortex-A53 devices are allowed to skip this flag.
if (Arch == llvm::Triple::aarch64 && (isAndroid || isOHOSFamily)) {
@@ -765,7 +770,8 @@ void tools::gnutools::Assembler::ConstructJob(Compilation &C,
}
case llvm::Triple::riscv32:
case llvm::Triple::riscv64: {
- StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
+ const llvm::Triple &Triple = getToolChain().getTriple();
+ StringRef ABIName = riscv::getRISCVABI(Args, Triple);
CmdArgs.push_back("-mabi");
CmdArgs.push_back(ABIName.data());
std::string MArchName =
@@ -774,6 +780,10 @@ void tools::gnutools::Assembler::ConstructJob(Compilation &C,
CmdArgs.push_back(Args.MakeArgString(MArchName));
if (!Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
Args.addOptOutFlag(CmdArgs, options::OPT_mrelax, options::OPT_mno_relax);
+
+ if (Triple.getVendor() == llvm::Triple::MipsTechnologies)
+ CmdArgs.push_back("-EL");
+
break;
}
case llvm::Triple::sparc:
@@ -1824,9 +1834,19 @@ static void findRISCVBareMetalMultilibs(const Driver &D,
.flag(Twine("-march=", Element.march).str())
.flag(Twine("-mabi=", Element.mabi).str()));
}
+ SmallVector<MultilibBuilder, 2> Endian;
+ bool IsMIPS = TargetTriple.getVendor() == llvm::Triple::MipsTechnologies;
+ if (IsMIPS) {
+ Endian.push_back(
+ MultilibBuilder("/riscv").flag("-EL").flag("-EB", /*Disallow=*/true));
+ Endian.push_back(
+ MultilibBuilder("/riscveb").flag("-EB").flag("-EL", /*Disallow=*/true));
+ }
MultilibSet RISCVMultilibs =
MultilibSetBuilder()
.Either(Ms)
+ .Either(Endian)
+ .Either(ArrayRef<MultilibBuilder>(Ms))
.makeMultilibSet()
.FilterOut(NonExistent)
.setFilePathsCallback([](const Multilib &M) {
@@ -1850,6 +1870,8 @@ static void findRISCVBareMetalMultilibs(const Driver &D,
}
}
+ addMultilibFlag(IsMIPS, "-EL", Flags);
+
if (selectRISCVMultilib(D, RISCVMultilibs, MArch, Flags,
Result.SelectedMultilibs))
Result.Multilibs = RISCVMultilibs;
@@ -1874,8 +1896,18 @@ static void findRISCVMultilibs(const Driver &D,
MultilibBuilder("lib64/lp64f").flag("-m64").flag("-mabi=lp64f");
MultilibBuilder Lp64d =
MultilibBuilder("lib64/lp64d").flag("-m64").flag("-mabi=lp64d");
+
+ SmallVector<MultilibBuilder, 2> Endian;
+ if (TargetTriple.getVendor() == llvm::Triple::MipsTechnologies) {
+ Endian.push_back(
+ MultilibBuilder("/riscv").flag("-EL").flag("-EB", /*Disallow=*/true));
+ Endian.push_back(
+ MultilibBuilder("/riscveb").flag("-EB").flag("-EL", /*Disallow=*/true));
+ }
+
MultilibSet RISCVMultilibs =
MultilibSetBuilder()
+ .Either(Endian)
.Either({Ilp32, Ilp32f, Ilp32d, Lp64, Lp64f, Lp64d})
.makeMultilibSet()
.FilterOut(NonExistent);
@@ -1883,6 +1915,7 @@ static void findRISCVMultilibs(const Driver &D,
Multilib::flags_list Flags;
bool IsRV64 = TargetTriple.getArch() == llvm::Triple::riscv64;
StringRef ABIName = tools::riscv::getRISCVABI(Args, TargetTriple);
+ bool IsMIPS = TargetTriple.getVendor() == llvm::Triple::MipsTechnologies;
addMultilibFlag(!IsRV64, "-m32", Flags);
addMultilibFlag(IsRV64, "-m64", Flags);
@@ -1892,6 +1925,7 @@ static void findRISCVMultilibs(const Driver &D,
addMultilibFlag(ABIName == "lp64", "-mabi=lp64", Flags);
addMultilibFlag(ABIName == "lp64f", "-mabi=lp64f", Flags);
addMultilibFlag(ABIName == "lp64d", "-mabi=lp64d", Flags);
+ addMultilibFlag(IsMIPS, "-EL", Flags);
if (RISCVMultilibs.select(D, Flags, Result.SelectedMultilibs))
Result.Multilibs = RISCVMultilibs;
@@ -2517,7 +2551,8 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
"riscv32-unknown-elf"};
static const char *const RISCV64LibDirs[] = {"/lib64", "/lib"};
static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
- "riscv64-unknown-elf"};
+ "riscv64-unknown-elf",
+ "riscv64-mti-elf"};
static const char *const SPARCv8LibDirs[] = {"/lib32", "/lib"};
static const char *const SPARCv8Triples[] = {"sparc-linux-gnu",
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index 1e9bd3de75f04..5d41510bf32e5 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -283,11 +283,13 @@ Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
const bool IsHexagon = Arch == llvm::Triple::hexagon;
const bool IsRISCV = Triple.isRISCV();
const bool IsCSKY = Triple.isCSKY();
+ const bool IsMipsSysRoot = IsMips ||
+ (IsRISCV && Triple.getVendor() == llvm::Triple::MipsTechnologies);
if (IsCSKY && !SelectedMultilibs.empty())
SysRoot = SysRoot + SelectedMultilibs.back().osSuffix();
- if ((IsMips || IsCSKY) && !SysRoot.empty())
+ if ((IsMipsSysRoot || IsCSKY) && !SysRoot.empty())
ExtraOpts.push_back("--sysroot=" + SysRoot);
// Do not use 'gnu' hash style for Mips targets because .gnu.hash
@@ -414,7 +416,11 @@ std::string Linux::computeSysRoot() const {
return std::string();
}
- if (!GCCInstallation.isValid() || !getTriple().isMIPS())
+ const bool IsMipsSysRoot = getTriple().isMIPS() ||
+ (getTriple().isRISCV() &&
+ getTriple().getVendor() == llvm::Triple::MipsTechnologies);
+
+ if (!GCCInstallation.isValid() || !IsMipsSysRoot)
return std::string();
// Standalone MIPS toolchains use different names for sysroot folder
@@ -424,8 +430,19 @@ std::string Linux::computeSysRoot() const {
const StringRef InstallDir = GCCInstallation.getInstallPath();
const StringRef TripleStr = GCCInstallation.getTriple().str();
const Multilib &Multilib = GCCInstallation.getMultilib();
+ std::string Path;
+ if (getTriple().isRISCV()) {
+ Path =
+ (InstallDir + "/../../../../sysroot" + Multilib.osSuffix() + "/../..")
+ .str();
+
+ if (getVFS().exists(Path))
+ return Path;
+
+ return std::string();
+ }
- std::string Path =
+ Path =
(InstallDir + "/../../../../" + TripleStr + "/libc" + Multilib.osSuffix())
.str();
diff --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 624099d21ae12..cf26b7eef1c63 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -173,6 +173,9 @@ void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
CmdArgs.push_back("-X");
+ if (ToolChain.getTriple().getVendor() == llvm::Triple::MipsTechnologies)
+ CmdArgs.push_back("-EL");
+
std::string Linker = getToolChain().GetLinkerPath();
bool WantCRTs =
@@ -229,4 +232,5 @@ void RISCV::Linker::ConstructJob(Compilation &C, const JobAction &JA,
JA, *this, ResponseFileSupport::AtFileCurCP(), Args.MakeArgString(Linker),
CmdArgs, Inputs, Output));
}
+
// RISCV tools end.
diff --git a/llvm/lib/Target/RISCV/RISCVSubtarget.cpp b/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
index 3c996c82fcec4..30aa8c267b484 100644
--- a/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
+++ b/llvm/lib/Target/RISCV/RISCVSubtarget.cpp
@@ -79,8 +79,17 @@ RISCVSubtarget::initializeSubtargetDependencies(const Triple &TT, StringRef CPU,
StringRef ABIName) {
// Determine default and user-specified characteristics
bool Is64Bit = TT.isArch64Bit();
- if (CPU.empty() || CPU == "generic")
- CPU = Is64Bit ? "generic-rv64" : "generic-rv32";
+ if (CPU.empty() || CPU == "generic") {
+ if (Is64Bit) {
+ if (TT.getVendor() == llvm::Triple::MipsTechnologies) {
+ CPU = "p8700";
+ } else {
+ CPU = "generic-rv64";
+ }
+ } else {
+ CPU = "generic-rv32";
+ }
+ }
if (TuneCPU.empty())
TuneCPU = CPU;
diff --git a/llvm/test/CodeGen/RISCV/rv64i-exhaustive-w-insts.ll b/llvm/test/CodeGen/RISCV/rv64i-exhaustive-w-insts.ll
index dad20b2d19464..eba4de25b8079 100644
--- a/llvm/test/CodeGen/RISCV/rv64i-exhaustive-w-insts.ll
+++ b/llvm/test/CodeGen/RISCV/rv64i-exhaustive-w-insts.ll
@@ -1684,7 +1684,7 @@ define zeroext i32 @zext_addiw_sext(i32 signext %a) nounwind {
; RV64ZBA-LABEL: zext_addiw_sext:
; RV64ZBA: # %bb.0:
; RV64ZBA-NEXT: addi a0, a0, 8
-; RV64ZBA-NEXT: zext.w a0, a0
+; RV64ZBA-NEXT: zext.w a0, a0
; RV64ZBA-NEXT: ret
%1 = add i32 %a, 8
ret i32 %1
@@ -1701,7 +1701,7 @@ define zeroext i32 @zext_addiw_zext(i32 zeroext %a) nounwind {
; RV64ZBA-LABEL: zext_addiw_zext:
; RV64ZBA: # %bb.0:
; RV64ZBA-NEXT: addi a0, a0, 9
-; RV64ZBA-NEXT: zext.w a0, a0
+; RV64ZBA-NEXT: zext.w a0, a0
; RV64ZBA-NEXT: ret
%1 = add i32 %a, 9
ret i32 %1
|
djtodoro
force-pushed
the
pr/mti-riscv-driver
branch
from
44fd7ad to
fa3aa23
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
djtodoro
force-pushed
the
pr/mti-riscv-driver
branch
from
fa3aa23 to
8c1ca6a
Compare
|
No tests? |
@topperc Added. |
ilovepi
left a comment
ilovepi
left a comment
There was a problem hiding this comment.
LGTM. Probably give @mshockwave a chance to weigh in before landing, though.
| } | ||
|
|
||
| if (Triple.isRISCV() && Triple.getVendor() == llvm::Triple::MipsTechnologies) | ||
| CmdArgs.push_back("-EL"); |
There was a problem hiding this comment.
This one confuses me. Why do you need it when non-MIPS doesn't?
| Args.addOptOutFlag(CmdArgs, options::OPT_mrelax, options::OPT_mno_relax); | ||
|
|
||
| if (Triple.getVendor() == llvm::Triple::MipsTechnologies) | ||
| CmdArgs.push_back("-EL"); |
| bool IsMIPS = TargetTriple.getVendor() == llvm::Triple::MipsTechnologies; | ||
| if (IsMIPS) { | ||
| Endian.push_back( | ||
| MultilibBuilder("/riscv").flag("-EL").flag("-EB", /*Disallow=*/true)); |
| MultilibSetBuilder() | ||
| .Either(Ms) | ||
| .Either(Endian) | ||
| .Either(ArrayRef<MultilibBuilder>(Ms)) |
There was a problem hiding this comment.
What's this got to do with MIPS?
| } | ||
| } | ||
|
|
||
| addMultilibFlag(IsMIPS, "-EL", Flags); |
There was a problem hiding this comment.
Or all of the rest of these... there are a lot of changes here that I don't understand the need for
| JA, *this, ResponseFileSupport::AtFileCurCP(), Args.MakeArgString(Linker), | ||
| CmdArgs, Inputs, Output)); | ||
| } | ||
|
|
There was a problem hiding this comment.
There's a lot going on here that has absolutely no explanation (no comments in the code, and the commit message is a single sentence that tells me nothing of use), and it's doing multiple different things. There's changing default arch strings, which makes some sense (well, it makes sense for you to want, I just don't love that changing the vendor changes -march, it's already confusing enough that baremetal and Unix-y triples have different defaults, but that ship has sailed). There's a whole bunch of -EL/-EB flag handling which doesn't make sense, I don't understand why MIPS triples need it when all the other vendors don't. Then there's extra sysroot stuff where again I don't understand why there are cases where only MIPS wants --sysroot= to work. Plus unexplained multilib changes.
I really don't want to see a proliferation of undocumented vendor-specific quirks. If there are things the generic RISC-V code currently omits then those should be added via standalone, well-documented (including in the commit message) commits. If there are things that need to be vendor-specific then they need explaining why they are there.
| Endian.push_back( | ||
| MultilibBuilder("/riscv").flag("-EL").flag("-EB", /*Disallow=*/true)); | ||
| Endian.push_back( | ||
| MultilibBuilder("/riscveb").flag("-EB").flag("-EL", /*Disallow=*/true)); |
There was a problem hiding this comment.
Shouldn't this be part of a non-vendor specific patch that adds big endian multilib support?
@jrtc27 Well, I do agree with the comment. We have some kind of an initial support for BE, and I left those lines there, which are NOOPs at the moment, but that can serve as a placeholder where we need to add BE related code in the future, but, I agree, it should have been added either with a decent comment, or not at all. Now that I think again, it introduces confusion only, so I will remove that part, and plan to post a new patch (or patch series) that will handle BE in a non-vendor specific way. Thanks! |
|
I will postpone this PR until I prepare big endian support, thank you all for the comments! |
7 participants
Add support for MipsTechnologies for RISC-V targets.