[PATCH] D44189: [RISCV] Verify the input value of -march=
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328690: [PATCH] [RISCV] Verify the input value of -march=
(authored by shiva, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44189?vs=139256=140049#toc
Repository:
rC Clang
https://reviews.llvm.org/D44189
Files:
lib/Driver/ToolChains/Arch/RISCV.cpp
test/Driver/riscv-arch.c
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -24,32 +24,93 @@
std::vector ) {
if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
StringRef MArch = A->getValue();
-// TODO: handle rv64
-std::pair MArchSplit = StringRef(MArch).split("rv32");
-if (!MArchSplit.second.size())
+if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+(MArch.size() < 5)) {
+ // ISA string must begin with rv32 or rv64.
+ // TODO: Improve diagnostic message.
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
return;
+}
-for (char c : MArchSplit.second) {
+// The canonical order specified in ISA manual.
+// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
+StringRef StdExts = "mafdc";
+
+bool HasF = false, HasD = false;
+char Baseline = MArch[4];
+
+// TODO: Add 'e' once backend supported.
+switch (Baseline) {
+default:
+ // First letter should be 'e', 'i' or 'g'.
+ // TODO: Improve diagnostic message.
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+ return;
+case 'i':
+ break;
+case 'g':
+ // g = imafd
+ StdExts = StdExts.drop_front(4);
+ Features.push_back("+m");
+ Features.push_back("+a");
+ Features.push_back("+f");
+ Features.push_back("+d");
+ HasF = true;
+ HasD = true;
+ break;
+}
+
+auto StdExtsItr = StdExts.begin();
+// Skip rvxxx
+StringRef Exts = MArch.substr(5);
+
+for (char c : Exts) {
+ // Check ISA extensions are specified in the canonical order.
+ while (StdExtsItr != StdExts.end() && *StdExtsItr != c)
+++StdExtsItr;
+
+ if (StdExtsItr == StdExts.end()) {
+// TODO: Improve diagnostic message.
+D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+return;
+ }
+
+ // Move to next char to prevent repeated letter.
+ ++StdExtsItr;
+
+ // The order is OK, then push it into features.
switch (c) {
- case 'i':
-break;
+ default:
+// TODO: Improve diagnostic message.
+D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+return;
case 'm':
Features.push_back("+m");
break;
case 'a':
Features.push_back("+a");
break;
case 'f':
Features.push_back("+f");
+HasF = true;
break;
case 'd':
Features.push_back("+d");
+HasD = true;
break;
case 'c':
Features.push_back("+c");
break;
}
}
+
+// Dependency check
+// It's illegal to specify the 'd' (double-precision floating point)
+// extension without also specifying the 'f' (single precision
+// floating-point) extension.
+// TODO: Improve diagnostic message.
+if (HasD && !HasF)
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
}
}
Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -0,0 +1,89 @@
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 |
[PATCH] D44189: [RISCV] Verify the input value of -march=
apazos added inline comments. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:51 + break; +case 'g': + // g = imafd One more question - how about non-standard extensions (vendor/custom) prefixed with X? Shouldn't we add the logic to process 'Xext' occurrences in the march string as part of this patch? Repository: rC Clang https://reviews.llvm.org/D44189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
asb accepted this revision. asb added a comment. This revision is now accepted and ready to land. Thanks for this Kito. A tiny formatting nit, but otherwise this looks good to me. Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:70 + while (StdExtsItr != StdExts.end() && *StdExtsItr != c) + ++StdExtsItr; + Indent should be 2 spaces rather than 3. Repository: rC Clang https://reviews.llvm.org/D44189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng marked 4 inline comments as done.
kito-cheng added a comment.
Hi Alex:
Thanks for your input, check for repeated letter was missed in my last patch :)
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:34
+
+// The canonical order specified in ISA manual.
+StringRef StdExts = "mafdc";
asb wrote:
> I'd reference Table 22.1 in RISC-V User-Level ISA V2.2 for anyone who wants
> to verify this.
Done.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:37-38
+
+bool hasF = false, hasD = false;
+char baseline = MArch[4];
+
asb wrote:
> Should be HasF, HasD, and Baseline to conform to standard LLVM naming
> conventions.
Fixed.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:65
+for (char c : Exts) {
+ // Check march is satisfied the canonical order.
+ while (StdExtsItr != StdExts.end() && *StdExtsItr != c)
asb wrote:
> I'd phrase this as "Check ISA extensions are specified in the canonical
> order."
Done.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:99
+
+// Dependency check
+if (hasD && !hasF)
asb wrote:
> I'd be tempted to give a bit more explanation a bit more "It's illegal to
> specify the 'd' (double-precision floating point) extension without also
> specifying the 'f' (single precision floating-point) extension".
Done.
Repository:
rC Clang
https://reviews.llvm.org/D44189
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng updated this revision to Diff 139256. kito-cheng added a comment. Update revision according Alex's review. Changes: - Add testcase for uppercase of -march string. - Add testcase for repeated letter in -march. - Add more comment. - Add several TODO item for diagnostic message improvement. - Fix coding style issue. Repository: rC Clang https://reviews.llvm.org/D44189 Files: lib/Driver/ToolChains/Arch/RISCV.cpp test/Driver/riscv-arch.c Index: test/Driver/riscv-arch.c === --- /dev/null +++ test/Driver/riscv-arch.c @@ -0,0 +1,89 @@ +// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// CHECK-NOT: error: invalid arch name ' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s +// RV32: error: invalid arch name 'rv32' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s +// RV32M: error: invalid arch name 'rv32m' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s +// RV32ID: error: invalid arch name 'rv32id' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s +// RV32L: error: invalid arch name 'rv32l' + +//
[PATCH] D44189: [RISCV] Verify the input value of -march=
asb added a comment.
Thanks for submitting this Kito. I've added some minor in-line comments. It
might also be worth adding a couple of extra cases to the tests:
- Repeated letters in the ISA string (e.g. rv32immafd)
- Upper case letters in the ISA string. We currently reject these (as does
GCC). It would be worth having a test that tracks this behaviour
We could probably give more informative error messages than just "invalid arch
name". Especially for common errors like -march=rv32. If not adding such
diagnostics in this patch, it would be good to add a `// TODO` note to record
that we'd like to do better.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:34
+
+// The canonical order specified in ISA manual.
+StringRef StdExts = "mafdc";
I'd reference Table 22.1 in RISC-V User-Level ISA V2.2 for anyone who wants to
verify this.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:37-38
+
+bool hasF = false, hasD = false;
+char baseline = MArch[4];
+
Should be HasF, HasD, and Baseline to conform to standard LLVM naming
conventions.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:65
+for (char c : Exts) {
+ // Check march is satisfied the canonical order.
+ while (StdExtsItr != StdExts.end() && *StdExtsItr != c)
I'd phrase this as "Check ISA extensions are specified in the canonical order."
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:99
+
+// Dependency check
+if (hasD && !hasF)
I'd be tempted to give a bit more explanation a bit more "It's illegal to
specify the 'd' (double-precision floating point) extension without also
specifying the 'f' (single precision floating-point) extension".
https://reviews.llvm.org/D44189
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng updated this revision to Diff 137970. kito-cheng added a comment. Add test cases for the correct inputs. https://reviews.llvm.org/D44189 Files: lib/Driver/ToolChains/Arch/RISCV.cpp test/Driver/riscv-arch.c Index: test/Driver/riscv-arch.c === --- /dev/null +++ test/Driver/riscv-arch.c @@ -0,0 +1,77 @@ +// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32im -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32ima -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32ic -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32ia -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32iac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv32-unknown-elf -march=rv32g -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv32-unknown-elf -march=rv32gc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64im -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64ima -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64ic -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64imafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64ia -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iaf -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafd -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64iac -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafc -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64iafdc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// RUN: %clang -target riscv64-unknown-elf -march=rv64g -### %s -fsyntax-only 2>&1 | FileCheck %s +// RUN: %clang -target riscv64-unknown-elf -march=rv64gc -### %s -fsyntax-only 2>&1 | FileCheck %s + +// CHECK-NOT: error: invalid arch name ' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s +// RV32: error: invalid arch name 'rv32' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s +// RV32M: error: invalid arch name 'rv32m' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s +// RV32ID: error: invalid arch name 'rv32id' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s +// RV32L: error: invalid arch name 'rv32l' + +// RUN: %clang -target riscv32-unknown-elf -march=rv32imadf -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMADF %s +// RV32IMADF: error: invalid arch name 'rv32imadf' + +// RUN: %clang -target riscv64-unknown-elf -march=rv64 -###
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng updated this revision to Diff 137687.
kito-cheng added a comment.
This version only update variable name which changed in last version by
accident.
https://reviews.llvm.org/D44189
Files:
lib/Driver/ToolChains/Arch/RISCV.cpp
test/Driver/riscv-arch.c
Index: test/Driver/riscv-arch.c
===
--- /dev/null
+++ test/Driver/riscv-arch.c
@@ -0,0 +1,29 @@
+// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s
+// RV32: error: invalid arch name 'rv32'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s
+// RV32M: error: invalid arch name 'rv32m'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s
+// RV32ID: error: invalid arch name 'rv32id'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
+// RV32L: error: invalid arch name 'rv32l'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imadf -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMADF %s
+// RV32IMADF: error: invalid arch name 'rv32imadf'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64 %s
+// RV64: error: invalid arch name 'rv64'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64M %s
+// RV64M: error: invalid arch name 'rv64m'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64ID %s
+// RV64ID: error: invalid arch name 'rv64id'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64L %s
+// RV64L: error: invalid arch name 'rv64l'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imadf -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMADF %s
+// RV64IMADF: error: invalid arch name 'rv64imadf'
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -24,32 +24,81 @@
std::vector ) {
if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
StringRef MArch = A->getValue();
-// TODO: handle rv64
-std::pair MArchSplit = StringRef(MArch).split("rv32");
-if (!MArchSplit.second.size())
+if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+(MArch.size() < 5)) {
+ // ISA string must begin with rv32 or rv64.
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
return;
+}
+
+// The canonical order specified in ISA manual.
+StringRef StdExts = "mafdc";
+
+bool hasF = false, hasD = false;
+char baseline = MArch[4];
+
+// TODO: Add 'e' once backend supported.
+switch (baseline) {
+default:
+ // First letter should be 'e', 'i' or 'g'.
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+ return;
+case 'i':
+ break;
+case 'g':
+ // g = imafd
+ StdExts = StdExts.drop_front(4);
+ Features.push_back("+m");
+ Features.push_back("+a");
+ Features.push_back("+f");
+ Features.push_back("+d");
+ hasF = true;
+ hasD = true;
+ break;
+}
-for (char c : MArchSplit.second) {
+auto StdExtsItr = StdExts.begin();
+// Skip rvxxx
+StringRef Exts = MArch.substr(5);
+
+for (char c : Exts) {
+ // Check march is satisfied the canonical order.
+ while (StdExtsItr != StdExts.end() && *StdExtsItr != c)
+ ++StdExtsItr;
+
+ if (StdExtsItr == StdExts.end()) {
+D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+return;
+ }
+
+ // The order is OK, then push it into features.
switch (c) {
- case 'i':
-break;
+ default:
+D.Diag(diag::err_drv_invalid_arch_name) << MArch;
+return;
case 'm':
Features.push_back("+m");
break;
case 'a':
Features.push_back("+a");
break;
case 'f':
Features.push_back("+f");
+hasF = true;
break;
case 'd':
Features.push_back("+d");
+hasD = true;
break;
case 'c':
Features.push_back("+c");
break;
}
}
+
+// Dependency check
+if (hasD && !hasF)
+ D.Diag(diag::err_drv_invalid_arch_name) << MArch;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng marked 2 inline comments as done.
kito-cheng added inline comments.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:48
+ break;
+default:
+ // First letter should be 'i' or 'g'.
apazos wrote:
> In the switch cases move default to first position.
Done :)
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:60
case 'm':
Features.push_back("+m");
break;
apazos wrote:
> So the subsequent features can appear in any order?
Yeah, here is a canonical order specified in ISA manual, I've check the order
now.
Repository:
rC Clang
https://reviews.llvm.org/D44189
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng updated this revision to Diff 137547.
Repository:
rC Clang
https://reviews.llvm.org/D44189
Files:
lib/Driver/ToolChains/Arch/RISCV.cpp
test/Driver/riscv-arch.c
Index: test/Driver/riscv-arch.c
===
--- /dev/null
+++ test/Driver/riscv-arch.c
@@ -0,0 +1,29 @@
+// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s
+// RV32: error: invalid arch name 'rv32'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s
+// RV32M: error: invalid arch name 'rv32m'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s
+// RV32ID: error: invalid arch name 'rv32id'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
+// RV32L: error: invalid arch name 'rv32l'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32imadf -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMADF %s
+// RV32IMADF: error: invalid arch name 'rv32imadf'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64 %s
+// RV64: error: invalid arch name 'rv64'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64M %s
+// RV64M: error: invalid arch name 'rv64m'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64ID %s
+// RV64ID: error: invalid arch name 'rv64id'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64L %s
+// RV64L: error: invalid arch name 'rv64l'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64imadf -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMADF %s
+// RV64IMADF: error: invalid arch name 'rv64imadf'
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -23,33 +23,82 @@
void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
std::vector ) {
if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
-// TODO: handle rv64
-std::pair MArchSplit = StringRef(MArch).split("rv32");
-if (!MArchSplit.second.size())
+StringRef March = A->getValue();
+if (!(March.startswith("rv32") || March.startswith("rv64")) ||
+(March.size() < 5)) {
+ // ISA string must begin with rv32 or rv64.
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
return;
+}
+
+// The canonical order specified in ISA manual.
+StringRef StdExts = "mafdc";
+
+bool hasF = false, hasD = false;
+char baseline = March[4];
+
+// TODO: Handle 'e' once backend supported.
+switch (baseline) {
+default:
+ // First letter should be 'e', 'i' or 'g'.
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
+ return;
+case 'i':
+ break;
+case 'g':
+ // g = imafd
+ StdExts = StdExts.drop_front(4);
+ Features.push_back("+m");
+ Features.push_back("+a");
+ Features.push_back("+f");
+ Features.push_back("+d");
+ hasF = true;
+ hasD = true;
+ break;
+}
-for (char c : MArchSplit.second) {
+auto StdExtsItr = StdExts.begin();
+// Skip rvxxx
+StringRef Exts = March.substr(5);
+
+for (char c : Exts) {
+ // Check march is satisfied the canonical order.
+ while (StdExtsItr != StdExts.end() && *StdExtsItr != c)
+ ++StdExtsItr;
+
+ if (StdExtsItr == StdExts.end()) {
+D.Diag(diag::err_drv_invalid_arch_name) << March;
+return;
+ }
+
+ // The order is OK, then push it into features.
switch (c) {
- case 'i':
-break;
+ default:
+D.Diag(diag::err_drv_invalid_arch_name) << March;
+return;
case 'm':
Features.push_back("+m");
break;
case 'a':
Features.push_back("+a");
break;
case 'f':
Features.push_back("+f");
+hasF = true;
break;
case 'd':
Features.push_back("+d");
+hasD = true;
break;
case 'c':
Features.push_back("+c");
break;
}
}
+
+// Dependency check
+if (hasD && !hasF)
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
apazos added inline comments.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:48
+ break;
+default:
+ // First letter should be 'i' or 'g'.
In the switch cases move default to first position.
Comment at: lib/Driver/ToolChains/Arch/RISCV.cpp:60
case 'm':
Features.push_back("+m");
break;
So the subsequent features can appear in any order?
Repository:
rC Clang
https://reviews.llvm.org/D44189
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44189: [RISCV] Verify the input value of -march=
kito-cheng created this revision.
kito-cheng added reviewers: asb, apazos.
Herald added subscribers: cfe-commits, shiva0217, niosHD, sabuasal,
jordy.potman.lists, simoncook, johnrusso, rbar.
This patch doing more check and verify the -march= string and will issue and
error if it's a invalid combination.
Repository:
rC Clang
https://reviews.llvm.org/D44189
Files:
lib/Driver/ToolChains/Arch/RISCV.cpp
test/Driver/riscv-arch.c
Index: test/Driver/riscv-arch.c
===
--- /dev/null
+++ test/Driver/riscv-arch.c
@@ -0,0 +1,23 @@
+// RUN: %clang -target riscv32-unknown-elf -march=rv32 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32 %s
+// RV32: error: invalid arch name 'rv32'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32M %s
+// RV32M: error: invalid arch name 'rv32m'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32ID %s
+// RV32ID: error: invalid arch name 'rv32id'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
+// RV32L: error: invalid arch name 'rv32l'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64 -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64 %s
+// RV64: error: invalid arch name 'rv64'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64m -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64M %s
+// RV64M: error: invalid arch name 'rv64m'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64id -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64ID %s
+// RV64ID: error: invalid arch name 'rv64id'
+
+// RUN: %clang -target riscv64-unknown-elf -march=rv64l -### %s -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64L %s
+// RV64L: error: invalid arch name 'rv64l'
Index: lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- lib/Driver/ToolChains/Arch/RISCV.cpp
+++ lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -23,33 +23,65 @@
void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
std::vector ) {
if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
-// TODO: handle rv64
-std::pair MArchSplit = StringRef(MArch).split("rv32");
-if (!MArchSplit.second.size())
+StringRef March = A->getValue();
+if (!(March.startswith("rv32") || March.startswith("rv64")) ||
+(March.size() < 5)) {
+ // ISA string must begin with rv32 or rv64.
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
return;
+}
+
+bool hasF = false, hasD = false;
+char baseline = March[4];
+
+switch (baseline) {
+case 'i':
+ break;
+case 'g':
+ Features.push_back("+m");
+ Features.push_back("+a");
+ Features.push_back("+f");
+ Features.push_back("+d");
+ hasF = true;
+ hasD = true;
+ break;
+default:
+ // First letter should be 'i' or 'g'.
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
+ break;
+}
-for (char c : MArchSplit.second) {
+// Skip rvxxx
+StringRef Exts = March.substr(5);
+
+for (char c : Exts) {
switch (c) {
- case 'i':
-break;
case 'm':
Features.push_back("+m");
break;
case 'a':
Features.push_back("+a");
break;
case 'f':
Features.push_back("+f");
+hasF = true;
break;
case 'd':
Features.push_back("+d");
+hasD = true;
break;
case 'c':
Features.push_back("+c");
break;
+ default:
+D.Diag(diag::err_drv_invalid_arch_name) << March;
+break;
}
}
+
+// Dependency check
+if (hasD && !hasF)
+ D.Diag(diag::err_drv_invalid_arch_name) << March;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
