RFC: simd enabled functions (omp declare simd / elementals)

[Richard Biener]

On Fri, Nov 1, 2013 at 4:04 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> Hello gentlemen.  I'm CCing all of you, because each of you can provide
> valuable feedback to various parts of the compiler which I touch.  I have
> sprinkled love notes with your names throughout the post :).
>
> This is a patch against the gomp4 branch.  It provides initial support for
> simd-enabled functions which are "#pragma omp declare simd" in the OpenMP
> world and elementals in Cilk Plus nomenclature.  The parsing bits for OpenMP
> are already in trunk, but they are silently ignored.  This patch aims to
> remedy the situation.  The Cilk Plus parsing bits, OTOH, are not ready, but
> could trivially be adapted to use this infrastructure (see below).
>
> I would like to at least get this into the gomp4 branch for now, because I
> am accumulating far too many changes locally.
>
> The main idea is that for a simd annotated function, we can create one or
> more cloned vector variants of a scalar function that can later be used by
> the vectorizer.
>
> For a simple example with multiple returns...
>
> #pragma omp declare simd simdlen(4) notinbranch
> int foo (int a, int b)
> {
>   if (a == b)
>     return 555;
>   else
>     return 666;
> }
>
> ...we would generate with this patch (unoptimized):

Just a quick question, queueing the thread for later review (aww, queue
back at >100 threads again - I can't get any work done anymore :().

What does #pragma omp declare simd guarantee about memory
side-effects and memory use in the function?  That is, unless the
function can be safely annotated with the 'const' attribute the
whole thing is useless for the vectorizer.

Thanks,
Richard.

> foo.simdclone.0 (vector(4) int simd.4, vector(4) int simd.5)
> {
>   unsigned int iter.6;
>   int b.3[4];
>   int a.2[4];
>   int retval.1[4];
>   int _3;
>   int _5;
>   int _6;
>   vector(4) int _7;
>
>   <bb 2>:
>   a.2 = VIEW_CONVERT_EXPR<int[4]>(simd.4);
>   b.3 = VIEW_CONVERT_EXPR<int[4]>(simd.5);
>   iter.6_12 = 0;
>
>   <bb 3>:
>   # iter.6_9 = PHI <iter.6_12(2), iter.6_14(6)>
>   _5 = a.2[iter.6_9];
>   _6 = b.3[iter.6_9];
>   if (_5 == _6)
>     goto <bb 5>;
>   else
>     goto <bb 4>;
>
>   <bb 4>:
>
>   <bb 5>:
>   # _3 = PHI <555(3), 666(4)>
>   retval.1[iter.6_9] = _3;
>   iter.6_14 = iter.6_9 + 1;
>   if (iter.6_14 < 4)
>     goto <bb 6>;
>   else
>     goto <bb 7>;
>
>   <bb 6>:
>   goto <bb 3>;
>
>   <bb 7>:
>   _7 = VIEW_CONVERT_EXPR<vector(4) int>(retval.1);
>   return _7;
>
> }
>
> The new loop is properly created and annotated with loop->force_vect=true
> and loop->safelen set.
>
> A possible use may be:
>
> int array[1000];
> void bar ()
> {
>   int i;
>   for (i=0; i < 1000; ++i)
>     array[i] = foo(i, 123);
> }
>
> In which case, we would use the simd clone if available:
>
> bar ()
> {
>   vector(4) int vect_cst_.21;
>   vector(4) int vect_i_6.20;
>   vector(4) int * vectp_array.19;
>   vector(4) int * vectp_array.18;
>   vector(4) int vect_cst_.17;
>   vector(4) int vect__4.16;
>   vector(4) int vect_vec_iv_.15;
>   vector(4) int vect_cst_.14;
>   vector(4) int vect_cst_.13;
>   int stmp_var_.12;
>   int i;
>   unsigned int ivtmp_1;
>   int _4;
>   unsigned int ivtmp_7;
>   unsigned int ivtmp_20;
>   unsigned int ivtmp_21;
>
>   <bb 2>:
>   vect_cst_.13_8 = { 0, 1, 2, 3 };
>   vect_cst_.14_2 = { 4, 4, 4, 4 };
>   vect_cst_.17_13 = { 123, 123, 123, 123 };
>   vectp_array.19_15 = &array;
>   vect_cst_.21_5 = { 1, 1, 1, 1 };
>   goto <bb 4>;
>
>   <bb 3>:
>
>   <bb 4>:
>   # i_9 = PHI <i_6(3), 0(2)>
>   # ivtmp_1 = PHI <ivtmp_7(3), 1000(2)>
>   # vect_vec_iv_.15_11 = PHI <vect_vec_iv_.15_12(3), vect_cst_.13_8(2)>
>   # vectp_array.18_16 = PHI <vectp_array.18_17(3), vectp_array.19_15(2)>
>   # ivtmp_20 = PHI <ivtmp_21(3), 0(2)>
>   vect_vec_iv_.15_12 = vect_vec_iv_.15_11 + vect_cst_.14_2;
>   vect__4.16_14 = foo.simdclone.0 (vect_vec_iv_.15_11, vect_cst_.17_13);
>   _4 = 0;
>   MEM[(int *)vectp_array.18_16] = vect__4.16_14;
>   vect_i_6.20_19 = vect_vec_iv_.15_11 + vect_cst_.21_5;
>   i_6 = i_9 + 1;
>   ivtmp_7 = ivtmp_1 - 1;
>   vectp_array.18_17 = vectp_array.18_16 + 16;
>   ivtmp_21 = ivtmp_20 + 1;
>   if (ivtmp_21 < 250)
>     goto <bb 3>;
>   else
>     goto <bb 5>;
>
>   <bb 5>:
>   return;
>
> }
>
> That's the idea.
>
> Some of the ABI issues still need to be resolved (mangling for avx-512, what
> to do with non x86 architectures, what (if any) default clones will be
> created when no vector length is specified, etc etc), but the main
> functionality can be seen above.
>
> Uniform and linear parameters (which are passed as scalars) are still not
> handled.  Also, Jakub mentioned that with the current vectorizer we probably
> can't make good use of the inbranch/masked clones.  I have a laundry list of
> missing things prepended by // FIXME if anyone is curious.
>
> I'd like some feedback from y'all in your respective areas, since this
> touches a few places besides OpenMP.  For instance...
>
> [Honza] Where do you suggest I place a list of simd clones for a particular
> (scalar) function?  Right now I have added a simdclone_of field in
> cgraph_node and am (temporarily) serially scanning all functions in
> get_simd_clone().  This is obviously inefficient.  I didn't know whether to
> use the current next_sibling_clone/etc fields or create my own.  I tried
> using clone_of, and that caused some havoc so I'd like some feedback.
>
> [Martin] I have adapted the ipa_parm_adjustment infrastructure to allow
> adding new arguments out of the blue like you mentioned was missing in
> ipa-prop.h.  I have also added support for creating vectors of arguments.
> Could you take a look at my changes to ipa-prop.[ch]?
>
> [Martin] I need to add new arguments in the case of inbranch clones, which
> add an additional vector with a mask as the last argument:  For the
> following:
>
> #pragma omp declare simd simdlen(4) inbranch
> int foo (int a)
> {
>   return a + 1234;
> }
>
> ...we would generate a clone with:
>
> vector(4) int
> foo.simdclone.0 (vector(4) int simd.4, vector(4) int mask.5)
>
> I thought it best to enhance ipa_modify_formal_parameters() and associated
> machinery than to add the new argument ad-hoc.  We already have enough ways
> of doing tree and cgraph versioning in the compiler ;-).
>
> [Richi] I would appreciate feedback on the vectorizer and the infrastructure
> as a whole.  Do keep in mind that this is a work in progress :).
>
> [Balaji] This patch would provide the infrastructure that can be used by the
> Cilk Plus elementals.  When this is complete, all that would be missing is
> the parser.  You would have to tag the original function with "omp declare
> simd" and "cilk plus elemental" attributes.  See simd_clone_clauses_extract.
>
> [Jakub/rth]: As usual, valuable feedback on OpenMP and everything else is
> greatly appreciated.
>
> Oh yeah, there are many more changes that would ideally be needed in the
> vectorizer.
>
> Fire away!