simoncook added a comment.
I have a question about backwards compatibility with this patch. Clang 9 has
shipped with rvXXg/etc defaulting to ilp32/lp64 ABI, and no march meaning
rvXXi, with users having built objects with those defaults. When Clang 10
ships, users they now need to always use a mabi/march flag to keep the same
compatibility with their Clang 9 flows, the enabled extensions and ABI will be
changed under their feet without warning (or at least until they either hit a
linker error in the ABI case, or potentially an invalid instruction trap in the
march case).
If we're going to change the defaults, this patch should at least contain an
update to the release notes file, this way this change would be documented for
users.
================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:476
+ if (MArch.startswith_lower("rv32")) {
+ if (MArch.substr(4).contains_lower("d") ||
+ MArch.startswith_lower("rv32g"))
----------------
Won’t this break if the user specifies a X/Z extension that has a d in the
name, so eg rv32iXd will try to use the ilp32d abi by default? For future
proofing, I think we may need to do a full parse of the isa string to verify
that d does actually mean the standard D-extension
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69383/new/
https://reviews.llvm.org/D69383
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
