[PATCH] D116879: [llvm] Allow auto-vectorization of sincos() using libmvec

[Tim Schmielau via Phabricator via llvm-commits]

tim.schmielau added inline comments.


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-VF2-VF8.ll:362
+; CHECK-LABEL: vector.body
+; CHECK: call void @_ZGVbN2vvv_sincos(<2 x double> [[TMP4:%.*]], <2 x double*> [[TMP5:%.*]], <2 x double*> [[TMP6:%.*]])
+;
----------------
rengolin wrote:
> tim.schmielau wrote:
> > RKSimon wrote:
> > > Is this correct? This looks like it creates a sincos signature that takes vectors of pointers to doubles, but I expect most sincos vector implementations to actually use pointers to vectors of doubles. Something like:
> > > ```
> > > void @sincos(<2 x double>, <2 x double>*, <2 x double>*)
> > > ```
> > > I hit something almost identical here: https://llvm.org/PR38424
> > I stumbled over this as well. Unfortunately the [[ https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt | libmvec Vector ABI Spec ]] isn't particularly enlightening on the matter:
> > 
> > ```
> > 2.3. Element Data Type to Vector Data Type Mapping
> >  
> > The vector data types for parameters are selected depending on ISA, vector length, data type of original parameter, and parameter specification.
> > For uniform and linear parameters (detailed description could be found in [1]), the original data type is preserved.
> > For vector parameters, vector data types are selected by the compiler. The mapping from element data type to vector data type is described as below.
> > * The bit size of vector data type of parameter is computed as: 
> > size_of_vector_data_type = VLEN * sizeof(original_parameter_data_type) * 8
> > For instance, for SSE version of vector function with parameter data type "int":
> > If VLEN = 4, size_of_vector_data_type = 4 * 4 * 8 = 128 (bits), which means one argument of type __m128 to be passed.
> > * If the size_of_vector_data_type is greater than the width of the vector register, multiple vector registers are selected and the parameter will be passed in multiple vector registers.
> > For instance, for SSE version of vector function with parameter data type "int": If VLEN = 8, size_of_vector_data_type = 8 * 4 * 8 = 256 (bits), so the vector data type is __m256, which means 2 arguments of type __m128 are to be passed.
> > ```
> > 
> > I interpret that as the `vvv` part of the signature indicating the three scalar arguments as being duplicated inside vector registers, which would make the last two arguments vectors of pointers, rather than pointers to vectors. I also tested that the generated code actually works with libmvec.
> > However, given the lack of specific mention of pointers in the vector ABI spec I don't feel particularly confident about my interpretation.
> Good catch! I totally missed that. Tim, how did you test this?
> 
> It's possible that vector of pointers "just worked" on X86 because it's supported, but this would probably break on non-SVE Arm. Regardless, that's the wrong implementation, we want just vectors.
> 
> Can you share the asm output of this sequence you're getting?
[un-inlining the discussion, as testcase + asm output are somewhat lengthy]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116879