[PATCH] D69976: [VFABI] Read/Write functions for the VFABI attribute.

[Francesco Petrogalli via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/Transforms/Utils/ModuleUtils.h:124
+void setVectorVariantNames(CallInst *CI,
+                           const SmallSet<std::string, 8> &VariantMappings);
+
----------------
fpetrogalli wrote:
> sdesmalen wrote:
> > Can these use StringRef?
> Indeed they can. Thank you.
It turned out that the assertion on invalid vector ABI names fire when running the code on the TLI tests in the loop vectorizer. It looks like the underlying memory of the StringRefs is modified. I have added some debugging and it shows that the mappings values are modified somewhere in the pipeline. I'll revert back to std::string for now.

FWIW, StringRef is not supposed to store data (see doxygen docs), but just to operate on buffers that owned by other component. This means that StringRef cannot be used as a return parameter of a function, because the buffer set up in the function is destroyed when leaving the function. We are not using StringRef as a return value here, but I guess that we  incur in a similar problem by using StringRef in a reference parameter that is in fact an output value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69976