Page MenuHomePhabricator

[LoopVectorize] Use OpenMP vector routines.
Needs ReviewPublic

Authored by fpetrogalli on Nov 30 2016, 1:53 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This patch enables the vectorizer to use the vector routines generated by clang with a OpenMP directive "#pragma omp declare simd".

The mapping "scalar name" -> "vector function" in the TargetLibraryInfo (TLI) is enriched with the vector function signature to account for the different vector function that can be associated to a scalar one by specifying "linear" or "uniform" parameter clauses in the "declare simd" directive.

Diff Detail

Event Timeline

fpetrogalli retitled this revision from to [LoopVectorize] Use OpenMP vector routines..
fpetrogalli updated this object.
fpetrogalli set the repository for this revision to rL LLVM.
fhahn added a subscriber: fhahn.Nov 30 2016, 2:39 AM
fpetrogalli removed rL LLVM as the repository for this revision.

Rebase plus clang format.

fhahn added inline comments.Nov 30 2016, 9:07 AM
lib/Analysis/TargetLibraryInfo.cpp
1194–1198

Can this be removed?

1343

It looks like the FTy argument is assigned to (line 1346) before it is used (line 1348). Is this intentional?

Just a high level question: why can't this extend the same insert mechanism we already use for the other vectorised functions, instead of creating a new one for OpenMP?

Just a high level question: why can't this extend the same insert mechanism we already use for the other vectorised functions, instead of creating a new one for OpenMP?

Hi Renato,

I am preparing a RFC email for the dev channel explaining my view on
this change. My short answer here is that the mapping ("scalar name" ,
"vectorization factor (VF)") into "vector name" that is currently
implemented in the TLI does not cover all the signatures of the
multiple vector variants that can be associated to a scalar function
by means of attaching "declare simd" directive to the function
declaration or definition.

Consider this example:

#pragma omp declare simd
#pragma omp declare simd uniform(y)
double pow(double x, double y)

In this case the user is asking the compiler to be aware of two vector
version available. With the current mapping it would generate two
entries for each VF (assuming VF = 4):

("pow", 4) -> "pow_vector_vector"
("pow", 4) -> "pow_vector_uniform_vector"

I made up the vector names, but you can see that with this mapping the
vectorizer doesn't have a way to choose the vector_vector version over
the vector_uniform_vector one. Of course, one could implement a "name
generation" in the vectorizer and search over the full triple
(scalar_name, VF, vector_name) until a match is found, but such
vector_name generator in the vectorizer should be target dependent, as
each architecture will come up with it's own vector ABI that mangle
the scalar name into the vector name.

With this solution one could rely in the vector signature of the
function using the multimap that maps scalar names into (vector names,
vector signatures).

In this case, the "pow" example before would generates this item in
the multimap:

"pow" -> [(pow_vector_vector, <double x 4>(<double x 4>,<double x 4>)),
          (pow_vector_uniform_vector, <double x 4>(<double x 4>, double))]

The "pow_vector_uniform_vector" here is clearly different because the
uniform parameter is kept as a scalar. This is just an example, things
can get even more complicated when both "linear" (and associated
modifiers "var", "uval", "val" and "ref") are used in directive.

With this multimap, the vectorizer wouldn't have to make up the vector
name for the matching. I think this is better because it skips the
need to write a target-dependent vector ABI name mangling method in
the vectorizer.

I am aware that the signature mapping method is not complete -
e.g. how do distinguish a step=2 from a step=3 in a linear clause of
the same parameter in the function signature. Nevertheless, even in
this situation using the function signature has the advantage of
providing a structure (the FunctionType) that can be used to attach
linear/uniform descriptors to vector or scalar parameters without
introducing new structures holding the parameter position.

I'll add this comment to the RFC, and extend it with more examples.

Cheers,

Francesco

I have removed all reviewers, this is not intended for code review.

Updates:

  1. rebase
  2. Fix bug in copy constructor of the TargetLibraryInfoImpl class - I have missed copying out the multimap needed in the algorithm
  3. Fix bug in the TLII::mangle method (now it returns a std::string instead of a StringRef)

No changes have been introduced in the scalar/vector function matching algorithm.

xtian added a subscriber: xtian.Jan 22 2017, 12:38 PM

All, as we discussed, we agree to follow the GCC VectorABI and be GCC VectorABI compatible, right? It could be happened, I am looking at an old patch. Thanks.

This is just a rebase with a recent llvm codebase.

All, as we discussed, we agree to follow the GCC VectorABI and be GCC VectorABI compatible, right? It could be happened, I am looking at an old patch. Thanks.

Hi Xinmin,

thanks for keeping an eye on this. I have updated the clang patch which now implements the GCC VectorABI - see https://reviews.llvm.org/D27250.

I understand this is not how the community has designed the work to be done around the implementation of "pragma omp declare simd" in the compiler: with my patch set one cannot tests each step of the compiler separately, as the information about the availability of vector function is passed directly from the front-end to the TargetLibraryInfo. What is really needed is something that writes the info in the IR - like the vector names as string attributes - so that the vectorizer in "opt" could be tested without the need of interfacing it with a particular front-end.

Now that I understand this need, I fully agree with you that we should use persistent information in the IR. I only wonder if there is some values in enabling my solution temporarily while the new proposed metadata for parallel metadata finds is way in ([RFC] IR-level Region Annotations [1]), as I think that the TargetLibraryInfo approach enables interfacing clang and teh vectorizer with external libraries.

[1] http://lists.llvm.org/pipermail/llvm-dev/2017-January/108906.html