[PATCH] D102394: [LoopVectorize] Don't attempt to widen certain calls for scalable vectors

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1708
+bool AArch64TTIImpl::isLegalScalableVectorIntrinsic(Intrinsic::ID IID) const {
+  switch (IID) {
+  case Intrinsic::sin:
----------------
sdesmalen wrote:
> I'd rather write this the other way around, have a list here of intrinsics we //do// support and can safely return 'true' for, and fall back on BaseT's implementation for the 'default' case (which seems to be missing here), which is more pessimistic and only returns true if it knows the intrinsics can always be vectorized, i.e. the trivially vectorizable cases.
So we do fall back on the BaseT version, it's just I didn't do it explicitly in the `default:` case. For some reason I thought this coding structure was the preferred way - I'm never really too sure about the coding style of switch statements in LLVM!

It's also worth pointing out that that the BaseT version calls isTriviallyVectorizable where even `sin` and `cos` will return true. Therefore, returning the BaseT version for those cases is not what we want. I could either:

1. Fill in the complete set of entries here with both the false and true cases, or
2. Change the BaseT version to always return false.

Any preferences?


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

https://reviews.llvm.org/D102394