[llvm-dev] [RFC] Re-implementing -fveclib with OpenMP

Fri Dec 14 09:05:01 PST 2018

Hi Renato,

Thank you for your review!

On Dec 12, 2018, at 3:58 PM, Renato Golin <renato.golin at linaro.org> wrote:

Hi Francesco,

This is a huge RFC and I don't think we can discuss all of it at the
same time, at least not in a constructive manner.

That’s why I was more on the idea to keep it in Phabricator, it would have been easier to track people comments (at least for me). :)

What ends up happening is that people ignore the thread and developers
get upset.

So, I'll start with the summary, to make sure the overall assumptions
in the RFC match the ones I have about it, then we can delve into
details.

I also think we should not discuss user-include files now. Whatever we
define for the standard ones will work for user driven ones, but user
driven have additional complexities that will only get in the way of
the standard discussion.

Comments inline...

On Wed, 12 Dec 2018 at 03:47, Francesco Petrogalli via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
Summary
=======

New `veclib` directives in clang

I know this is not new but, why "fveclib"? From the review, I take
this is the same as GCC's "mveclibabi", and if it is, why come up with
a new name for the same thing?

Although I see your reasoning around the compatibility with other compilers, I don’t this this is the place to discuss this. The -fveclib option was introduced prior to this RFC, and for now we have to leave with it. Whether we want to keep it or change it to a gcc compatible one, is not something we have to discuss here. In particular, I suspect that there are users of -fveclib that would shout in the mailing list  is we convert it to a new option, as it will break their build system. Again, not for this RFC discussion.

If it's not, what justifies implementing a different way of handling
the same concept (vector math libraries), which is surely going to
confuse a lot of users.

In some reviews, it was said that some proprietary compilers already
use "fveclib", but between being coherent with other OSS compilers and
closed source compilers, I think the answer is clear.

I'm not against the name, I'm just making sure we're not creating
problem for ourselves.

--------------------------------

1.  `#pragma veclib declare simd [clause, ]`, same as
   `#pragma omp declare simd` from OpenMP 4.0+.

Why not just use "pragma omp simd"?

If I recall correctly, there's an option to allow OMP SIMD pragmas
without enabling full OMP, so that we can use it without needing all
the headers and libraries, just to control vectorisation.

Creating new pragmas should be seen with extreme prejudice, as these
things tend to simplify the life of the compiler developers but create
nightmares for application developers, especially if they want to use
multiple compilers.

Yes, the idea was to use OpenMP pragmas only. From the discussion it turned out that OpenMP vectorization and function vectorization are two orthogonal problems (in the sense that we want to be able to turn on math function vectorization without enabling vectorization of the functions that users may mark as declare simd, and vice versa), so we decided to introduce something new (the veclib pragma). It is 100% compatible with the OpenMP one, so it minimizes the work needed in the compiler to support it, and at the same time it is based on a public standard, so I think it is the best choice we could do.

The section on the compatibility with OpenMP explain how -fopenmp-[simd] and -fveclib interacts.

2.  `#pragma omp declare variant`, same as `#pragma omp declare variant`
   restricted to the `simd` context selector, from OpenMP 5.0+.

Is this just for the user-driven stuff? If so, let's look at it later.

No - this is needed to be able to attach non standard names to the standard ones (see the example of the vector-variant attribute for SVML).

New `math.h` header file
------------------------

Shipped in `<clang>/lib/Headers/math.h`, contains all the declaration of
the functions available in the vector library `X`, `ifdef` guarded by
the macro `__CLANG_ENABLE_LIBRARY_X`.

So, the compiler will have the header files and the libraries will be
in charge of implementing them, to avoid linkage errors?

If this is a standard ABI that multiple libraries follow, I'm in
favour. If we'll end up with one (or more) header(s) per library or
worse, need to update the header every time the library changes
something, then I'm completely against.

The compiler doesn’t have control on the library - the behavior you are describing will always happen. The only advantage of storing in a header file with standard descriptor (the openmp based ones) is that it makes it easier to maintain and modify. The different sets are guarded by preprocessor macros, it could be done also with macros that are specific to version of the libraries.

The alternative is to require that the libraries are shipped with a header file with the descriptors of the vector version (OpnMP would be the best choice, because it is standard). Unfortunately, I don't think this is something that is going to happen (but I would be very happy to be proved wrong here!)

The latter will generate the compatibility issue I mentioned in one of
the reviews, where the compiler has different header files but the
implementations are slightly off-base. Keeping multiple copies of
those libraries in the same file system (for different users in the
same clusters) is even worse.

That's the kind of thing that is better left for the libraries
themselves. If they have both headers and objects, keeping all
together into one directory is enough.

We have to store the list of the available vector functions somewhere. Now it is done in the backend of LLVM, this RFC proposes to move it to the frontend, in a convenient way that will enable more vectorization opportunities by being compatible with what OpenMP provides.

Option behavior, and interaction with OpenMP
--------------------------------------------

`-fveclib=X`

:   The driver transform this into
   `-fparse-veclib -D__CLANG_ENABLE_LIBRARY_X=1 -lX`. This is used only
   for users that want to vectorize `math.h` functions.

Why not just include the header when you use it, instead of include
and guard for all cases?

Hum - I am not sure I understand what you re saying here. The idea is to keep user code as it is, with just #include <math.h>. If we come up with a set of library-specific header files shipped with the compiler, we we would have to -include them at command line, so that -fveclib=X would become -lX -include=path/to/X.h

`-fopenmp[-simd]`

:   No vectorization happens other then for those functions that are
   marked with OpenMP declare simd. The header `math.h` is loaded, but
   the `veclib` decorated declarations are invisible to the compiler
   instance because hidden behind the `__CLANG_ENABLE_LIBRARY_X`
   macros, which are not defined.

"No vectorisation" you mean, no "function" vectorisation. Other
vectorisation (from -O3 etc) will still happen.

Yes, I will fix it

`-fopenmp[-simd] -fveclib=X` or

: Same behavior as without the `-fopenmp[-simd]` option.

So, fveclib will enable OMP SIMD by default? I think that's what some
of the reviews (particularly on certification) were against. This is
not correct.

No, I think you got this wrong. -fveclib itself doesn't enable any OpenMP. OpenMP is enabled only when -fopenmp[-simd] is invoked.

The only way this can work is without including OMP dependencies when
using vector libraries. If the omp-simd option does not add OMP deps
(as I hinted above, there may be a way), then this is fine. But if
veclib flags force OMP dependencies, than this cannot work.

I think I haven't been clear enough on describing this last combination. Would it be better if I replace it with the following?

``-fopenmp[-simd] -fveclib=X`` or ``-fopenmp[-simd]
-fveclib-include=path/to/user/provided/header/file.h``

:   Same behavior as without the ``-fopenmp[-simd]`` option. In particular, both the "veclib" functions in math.h (or those in the user provided functions when -fveclib-include is used ) are available for vectorization, together with those marked by the OpenMP pragmas.

Francesco

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181214/a41fcbda/attachment-0001.html>