[PATCH] D44189: [RISCV] Verify the input value of -march=

[Kito Cheng via Phabricator via cfe-commits]

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