[PATCH] D142144: [RISCV][Driver] Add -rvv-vector-bits= option similar to -sve-vector-bits=

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:3587
+  HelpText<"Specify the size in bits of an RVV vector register. Defaults to the"
+           " vector length agnostic value of \"scalable\". Also accepts \"zvl\""
+           " to use the value implied by -march/-mcpu. (RISC-V only)">;
----------------
Same question as below with the value `"scalable"`. Should we excise it for now?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2150
+          Args.MakeArgString("-mvscale-min=" + llvm::Twine(VScaleMin)));
+    } else if (!Val.equals("scalable")) {
+      // Handle the unsupported values passed to mrvv-vector-bits.
----------------
Is this check right? I don't see mention of "scalable" in the commit description. Should this be a plain `else`?


================
Comment at: clang/test/Driver/riscv-rvv-vector-bits.c:2
+// -----------------------------------------------------------------------------
+// Tests for the -msve-vector-bits flag
+// -----------------------------------------------------------------------------
----------------
Need to adjust this comment


================
Comment at: clang/test/Driver/riscv-rvv-vector-bits.c:16
+// RUN: %clang -c %s -### -target riscv64-linux-gnu -march=rv64gc_zve64x \
+// RUN:  -mrvv-vector-bits=scalable 2>&1 | FileCheck --check-prefix=CHECK-SCALABLE %s
+
----------------
Another use of `"scalable"` which I'm not sure about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142144/new/

https://reviews.llvm.org/D142144