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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Thanks for all the changes @fpetrogalli, this patch is really taking shape! Just added a few more drive-by comments.



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:94
+/// \param PD -> ParsedData instance to store the results produced by the parse.
+bool tryParserVFABIName(const StringRef mangledName, ParsedData &PD);
+
----------------
lebedev.ri wrote:
> I'm not sure why this should not also return `Optional<ParsedData>`.
nit: Can we rename this main function to something like tryDemangleForVFABI ?


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:116
+  bool operator==(const VFShape &Other) const {
+    return std::tie(VF, ISA, Parameters, ScalarName, VectorName) ==
+           std::tie(Other.VF, Other.ISA, Other.Parameters, Other.ScalarName,
----------------
Given that ScalarName and VectorName are not part of the equality-comparison, does that imply they should not be part of VFShape ?


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:30
+  // Capture the ISA value (one char): `_ZGV<isa>...`.
+  const StringRef ISAValue = MangledName.substr(4, 1);
+
----------------
Why is it named `getISA` which starts parsing at position4 rather than `tryParseISA` which starts parsing at position 0?


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:314
+  // First, drop the first 6 chars of the mangled name: _ZGV<isa><mask>
+  StringRef ParseString = MangledName.drop_front(6);
+
----------------
Can we make the dropping of these substrings part of getMask and getISA (on success) ?


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:356
+
+  // If the scalar name is missing, bail out.
+  if (PD.ScalarName.empty())
----------------
nit: this comment seems unnecessary (the code says it all)


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

https://reviews.llvm.org/D66024