[clang] [llvm] Add clang driver changes to support MTI RISC-V (PR #134065)
Djordje Todorovic via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 8 01:17:39 PDT 2025
djtodoro wrote:
> 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.
@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!
https://github.com/llvm/llvm-project/pull/134065
More information about the cfe-commits
mailing list