asb added a comment. This has been hanging around for a while, but I think we'd basically agreed this is the right logic. The comments have ended up referring to flags that don't exist on Clang making it a little hard to follow, and I've added a request to slightly expand testing. If you make those cleanups I think it should be ready for a final review and merge.
As Sam says, lets flag this in today's RISC-V LLVM call to double-check everyone is happy. ================ Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:622 + // 1. Explicit choices using `--with-arch=` + // 2. Based on `-mcpu` if target cpu has default isa extension feature + // 3. A default based on `--with-abi=`, if provided ---------------- As clang has no with-arch or with-abi, this comment seems inaccurate? ================ Comment at: clang/test/Driver/riscv-cpus.c:2 +// Check target CPUs are correctly passed. + +// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=rocket-rv32 | FileCheck -check-prefix=MCPU-ROCKETCHIP32 %s ---------------- I think for completeness this test should be validating the interaction of the ABI choosing logic with CPU selection as well. With the implemented logic I believe it should show that lp64d is selected for -mcpu=sifive-u54 and that -mcpu=sifive-u54 -mabi=lp64 will respect the ABI choice Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71124/new/ https://reviews.llvm.org/D71124 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
