[PATCH] D140283: [RISCV] Move -riscv-v-vector-bits-max/min options to RISCVTargetMachine.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:132
   // riscv-v-vector-bits-max should be no less than it.
-  if (RVVVectorBitsMax < (int)ZvlLen)
+  if (RVVVectorBitsMax != 0 && RVVVectorBitsMax < ZvlLen)
     report_fatal_error("riscv-v-vector-bits-max specified is lower "
----------------
eopXD wrote:
> Maybe add comment that 0 is default value when `-riscv-v-vector-bits-max` is not set.
> 
> To be very paranoid, do we need check if user is specifying max to `-2`, which the option's datatype does allow?
We should detect -2 as being out of bounds in RISCVTargetMachine.cpp


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:159
+  Key += "RVVMin";
+  Key += std::to_string(RVVBitsMin);
+  Key += "RVVMax";
----------------
eopXD wrote:
> Maybe leverage Twine here?
I don't think Twine is useful here. SmallString doesn't have any methods that take a Twine. So we'd have to convert the Twine to a std::string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140283