[PATCH] D66024: [SVFS] Vector Function ABI name demangler.

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added inline comments.


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:75-82
+// \param VF -> holds <vlen> if <vlen> is a number.
+// \param IsMasked -> true if <mask> == "M", false if <mask> == "N".
+// \param IsScalable -> true if <vlen> == "x".
+// \param ISA -> maps to <isa>.
+// \param Parameters -> holds the <paramaters>.
+// \param ScalarName -> holds the <scalarname>.
+// \param VectorVarian -> holds <redirection> is present, otherwise holds
----------------
lebedev.ri wrote:
> VectorVariant
> 
> But, i think this function should return `llvm::Optional<Some Struct>`, or `error_or` maybe
I have changed the signature to use a single input parameter. The method is still returning a `bool`.

I have used optional in the static method that creates the `VFShape` instance.


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:104
+  if (ParamKind == VFParamKind::Invalid)
+    llvm_unreachable("Unexpected Parameter Kind");
+  return ParamKind;
----------------
lebedev.ri wrote:
> This is unreachable, right? Else i think this might need better error propagation strategy.
I have replace the output type with `Optional<VFParamKind>` and moved the `llvm_unreachable` at the end of the function.


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:164
+      // the case.
+      llvm_unreachable("Invalid Vector Function ABI name.");
+
----------------
lebedev.ri wrote:
> same, and elsewhere.
Done. The static method now returns an optional value.


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

https://reviews.llvm.org/D66024