[PATCH] D78054: [VFDatabase] Scalar functions are vector functions with VF =1

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

In D78054#2005917 <https://reviews.llvm.org/D78054#2005917>, @masoud.ataei wrote:

> Thanks Francesco for the reviews. 
>  The reviews are addressed.


Thank you! A couple more of comments, mostly nits, plus one substantial comment for the ll test, as I am not sure you are testing the right thing for this patch as it is right now (apologies if that was induced by my previous review).

Kind regards,

Francesco



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:102
+  static VFShape getScalarShape(const CallInst &CI) {
+    return VFShape::get(CI, {1, false}, false);
+  }
----------------
Please name the constant parameters:

```
 return VFShape::get(CI, /*EC*/{1, false}, /*HasGlobalPredicate*/false);
```


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:195
   const Module *M;
+  const CallInst &CI;
   /// List of vector functions descritors associated to the call
----------------
Docs: 

```
/// The CallInst instance being queried for scalar to vector mappings.
const CallInst &CI;
```


================
Comment at: llvm/test/Transforms/LoopVectorize/vectorizeVFone.ll:8
+; CHECK-LABEL: getScalarFunc
+; CHECK-NOT: call fast <{{[0-9]+}} x double> @{{.*}}atan{{.*}}(<{{[0-9]+}} x double> %{{[0-9]+}})
+entry:
----------------
Are you sure you are checking the right thing here? If vectorization happens with VF = 2, you wouldn't be catching this because the VFDatabase would be redirecting the VF=2 call  to `__atand2_massv`, as specified by the VFABI attribute below. If my understanding is correct, whaat you want to test here is that the compiler doesn't crash, and that it does not emit vector code. I think that should be tested by:

```
; CHECK-LABEL: getScalarFunc
; CHECK-NOT: <{{[0-9]+}} x double>
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78054