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

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added a comment.

In D66024#1669011 <https://reviews.llvm.org/D66024#1669011>, @sdesmalen wrote:

>




>> One more nit: while working on the SVFS, I have noticed that the `llvm_unreachable` I have added at the end of the static method to create FShape out of the parsed data was quite annoying: it got fired every time the CI I was trying to vectorize didn't have the name with the mangled attribute. hence, I have removed the assertion, and converted the `ASSERT_DEATH` tests into `EXPECT_FALSE` tests in `TEST(VectorFunctionABITests, OnlyValidNames)`. This should not be controversial (I am happy to be told I am wrong if that's the case).
> 
> That seems like the better choice given how you've shaped the interface that returns a `Optional<VFShape>`, but I don't really understand why this would break CI.

The way I am building the VFShape is as follows:

  const StringRef AttrString =
          CI->getCalledFunction()->getFnAttribute("vector-function-abi-variant").getValueAsString();
        SmallVector<StringRef, 8> ListOfStrings;
        AttrString.split(ListOfStrings, ",");
  SmallVector<VFShape, 8> Ret;
        for (auto MangledName : ListOfStrings) {
  	auto Shape = VFShape::getFromVFABI(MangledName);
          if (Shape.hasValue() && Shape.getValue().ScalarName == ScalarName)
            Ret.push_back(Shape.getValue());
        }

Most of the functions don't have the attribute, which result in list of string consisting of a single element, which is an empty string, therefore the parser fails and the exception is raised.

Maybe there is a better way to do it, but given the `Optional<VFShape>` return, it doesn't make sense to raise the exception on failure.


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

https://reviews.llvm.org/D66024