lenary marked an inline comment as done. lenary added a comment. I agree backwards compatibility is hard here.
- This method was introduced to have a single place to choose a default `march` string if none was chosen before. I think this change is useful (saves defaults being calculated in a multitude of other places, which means they may not agree). - The choice of a default march string was based, as closely as possible, on `config.gcc`. This may not be what we want (and is the source of backwards compatibility issues). - I 100% agree that a release notes entry is needed. I will write one just as soon as we finalize the default behaviour. - The elf multilib changes in D67508 <https://reviews.llvm.org/D67508> seem to be more complex, as `config.gcc` chooses a march/mabi default that is not built by `t-elf-multilib`. I think I would understand updating the logic to use fewer architecture extensions for a given ABI (i.e., default to `rv32i` unless someone asks for `ilp32f/d`) rather than more (`rv32gc` when someone asks for `ilp32`). This should be more backwards compatible, but also requires carefully ensuring the logic is not circular. At the very least, I do not plan to merge this patch until we have had a chance to discuss it next Thursday (31st Oct) on the RISC-V backend call. ================ 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")) ---------------- simoncook wrote: > 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 You're right that having "d" in any extension name will cause issues. The same is true for any gcc build today (9.2.0). 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
