[PATCH] D73286: [llvm][VectorUtils] Tweak VFShape for scalable vector functions.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:170
+Optional<VFInfo> tryDemangleForVFABI(StringRef MangledName,
+                                     const Module *M = nullptr);
 
----------------
fpetrogalli wrote:
> jdoerfert wrote:
> > Any reason one would not pass `M` here? If not, I would remove the default and maybe even make it required (via reference).
> Could do - that will require changing all unit tests to provide a module though... are you happy for me to do that?
I guess we can script it* so it should not be too bad work-wise. The question is, what do we want to achieve. If there is a use case that we want to support going forward which does not need the module, I'm fine with the pointer. If we are actually not anticipating such a use case we will just burden and confuse our future selves as we open up the possibility for all kinds of subtle bugs as someone forgot somewhere to pass a module, we should at least remove the default value.

* Something like
```
for file in $(ag --files-with-matches "tryDemangleForVFABI")
do
  sed -i -e 's:tryDemangleForVFABI(\([^)]*\)):tryDemangleForVFABI(\1, nullptr):' ${file}
done

```
(UNTESTED)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73286