[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

It would be nice if the LLVM interfaces could be updated as well (could also be done in a follow-up patch)

From:

  std::pair<unsigned, unsigned> getVScaleRangeArgs()

To:

  Optional<unsigned> getVScaleRangeMax();
  unsigned getVScaleRangeMin();

Since that would simplify some of the queries to this interface.



================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:597
+def err_cc1_unbounded_vscale_min : Error<
+  "minimum vscale cannot be unbounded (0)">;
 }
----------------
nit: `must be an unsigned integer than 0`


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4130
+  if (Arg *A = Args.getLastArg(options::OPT_mvscale_min_EQ)) {
+    if (StringRef(A->getValue()).equals("0"))
+      Diags.Report(diag::err_cc1_unbounded_vscale_min);
----------------
I'd suggest parsing the value as an integer, and checking that it's > 1. See `StringRef::getAsInteger()`.


================
Comment at: llvm/docs/LangRef.rst:2137-2138
     This attribute indicates the minimum and maximum vscale value for the given
-    function. A value of 0 means unbounded. If the optional max value is omitted
-    then max is set to the value of min. If the attribute is not present, no
-    assumptions are made about the range of vscale.
+    function. A value of 0 means unbounded, only the max value can be
+    unbounded. If the optional max value is omitted then max is set to the
+    value of min. If the attribute is not present, no assumptions are made
----------------
A maximum value of 0 means unbounded.



I'd also add that `min` must be greater than 0.


================
Comment at: llvm/lib/IR/Verifier.cpp:2064
+    if (Args.first == 0)
+      CheckFailed("'vscale_range' minimum cannot be unbounded", V);
+
----------------
must be greater than 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113294