search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
25.96k stars 10.6k forks source link

[llvm][RISCV] Improve error message for invalid extension letters #90468

Closed DavidSpickett closed 2 weeks ago

DavidSpickett commented 2 weeks ago

Previously you got: clang: error: invalid arch name 'rv64v', first letter should be 'e', 'i' or 'g'

Which to me, unfamiliar with riscv, reads as if I should have used "[eig]rv64v". Which is not what clang means.

Include the first bit in the error message to make this clearer: clang: error: invalid arch name 'rv64v', first letter after 'rv64' should be 'e', 'i' or 'g'

llvmbot commented 2 weeks ago

@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: David Spickett (DavidSpickett)

Changes Previously you got: clang: error: invalid arch name 'rv64v', first letter should be 'e', 'i' or 'g' Which to me, unfamiliar with riscv, reads as if I should have used "[eig]rv64v". Which is not what clang means. Include the first bit in the error message to make this clearer: clang: error: invalid arch name 'rv64v', first letter after 'rv64' should be 'e', 'i' or 'g' --- Full diff: https://github.com/llvm/llvm-project/pull/90468.diff 3 Files Affected: - (modified) clang/test/Driver/riscv-arch.c (+3-3) - (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+2-1) - (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+7-3) ``````````diff diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c index 8399b4e97f86d5..abbe8612b3780a 100644 --- a/clang/test/Driver/riscv-arch.c +++ b/clang/test/Driver/riscv-arch.c @@ -209,7 +209,7 @@ // RUN: not %clang --target=riscv32-unknown-elf -march=rv32q -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-LETTER %s // RV32-LETTER: error: invalid arch name 'rv32q', -// RV32-LETTER: first letter should be 'e', 'i' or 'g' +// RV32-LETTER: first letter after 'rv32' should be 'e', 'i' or 'g' // RUN: not %clang --target=riscv32-unknown-elf -march=rv32imcq -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ORDER %s @@ -239,12 +239,12 @@ // RUN: not %clang --target=riscv32-unknown-elf -march=rv32xabc -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32X %s // RV32X: error: invalid arch name 'rv32xabc', -// RV32X: first letter should be 'e', 'i' or 'g' +// RV32X: first letter after 'rv32' should be 'e', 'i' or 'g' // RUN: not %clang --target=riscv32-unknown-elf -march=rv32sabc -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32S %s // RV32S: error: invalid arch name 'rv32sabc', -// RV32S: first letter should be 'e', 'i' or 'g' +// RV32S: first letter after 'rv32' should be 'e', 'i' or 'g' // RUN: not %clang --target=riscv32-unknown-elf -march=rv32ix -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32X-NAME %s diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp index 494dc76a18521c..20182fb06037c2 100644 --- a/llvm/lib/TargetParser/RISCVISAInfo.cpp +++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp @@ -639,7 +639,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, switch (Baseline) { default: return createStringError(errc::invalid_argument, - "first letter should be 'e', 'i' or 'g'"); + "first letter after \'" + Arch.slice(0, 4) + + "\' should be 'e', 'i' or 'g'"); case 'e': case 'i': break; diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp index c945c4fbcf6352..9f23000d733d06 100644 --- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp +++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp @@ -120,10 +120,14 @@ TEST(ParseArchString, RejectsInvalidBaseISA) { EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()), "string must begin with rv32{i,e,g} or rv64{i,e,g}"); } - for (StringRef Input : {"rv32j", "rv64k", "rv32_i"}) { + + for (StringRef Input : {"rv32j", "rv32_i"}) { EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()), - "first letter should be 'e', 'i' or 'g'"); + "first letter after 'rv32' should be 'e', 'i' or 'g'"); } + + EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv64k", true).takeError()), + "first letter after 'rv64' should be 'e', 'i' or 'g'"); } TEST(ParseArchString, RejectsUnsupportedBaseISA) { @@ -395,7 +399,7 @@ TEST(ParseArchString, AcceptsAmbiguousFromRelaxExtensions) { TEST(ParseArchString, RejectsRelaxExtensionsNotStartWithEorIorG) { EXPECT_EQ( toString(RISCVISAInfo::parseArchString("rv32zba_im", true).takeError()), - "first letter should be 'e', 'i' or 'g'"); + "first letter after 'rv32' should be 'e', 'i' or 'g'"); } TEST(ParseArchString, ``````````