<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" dir="ltr" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, "EmojiFont", "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;">
<div dir="auto" class="ApplePlainTextBody" style="word-wrap:break-word">Hi Renato,
<div><br>
</div>
<div>Thank you for your review!<br>
<div><br>
<blockquote type="cite">On Dec 12, 2018, at 3:58 PM, Renato Golin <renato.golin@linaro.org> wrote:<br>
<br>
Hi Francesco,<br>
<br>
This is a huge RFC and I don't think we can discuss all of it at the<br>
same time, at least not in a constructive manner.<br>
<br>
</blockquote>
<div><br>
</div>
<div>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). :)</div>
<br>
<blockquote type="cite">What ends up happening is that people ignore the thread and developers<br>
get upset.<br>
<br>
So, I'll start with the summary, to make sure the overall assumptions<br>
in the RFC match the ones I have about it, then we can delve into<br>
details.<br>
<br>
I also think we should not discuss user-include files now. Whatever we<br>
define for the standard ones will work for user driven ones, but user<br>
driven have additional complexities that will only get in the way of<br>
the standard discussion.<br>
<br>
Comments inline...<br>
<br>
On Wed, 12 Dec 2018 at 03:47, Francesco Petrogalli via llvm-dev<br>
<llvm-dev@lists.llvm.org> wrote:<br>
<blockquote type="cite" class="">Summary<br>
=======<br>
<br>
New `veclib` directives in clang<br>
</blockquote>
<br>
I know this is not new but, why "fveclib"? From the review, I take<br>
this is the same as GCC's "mveclibabi", and if it is, why come up with<br>
a new name for the same thing?<br>
<br>
</blockquote>
<div><br>
</div>
<div>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.</div>
<br>
<blockquote type="cite">If it's not, what justifies implementing a different way of handling<br>
the same concept (vector math libraries), which is surely going to<br>
confuse a lot of users.<br>
<br>
In some reviews, it was said that some proprietary compilers already<br>
use "fveclib", but between being coherent with other OSS compilers and<br>
closed source compilers, I think the answer is clear.<br>
<br>
I'm not against the name, I'm just making sure we're not creating<br>
problem for ourselves.<br>
<br>
<br>
<blockquote type="cite" class="">--------------------------------<br>
<br>
1.  `#pragma veclib declare simd [clause, ]`, same as<br>
   `#pragma omp declare simd` from OpenMP 4.0+.<br>
</blockquote>
<br>
Why not just use "pragma omp simd"?<br>
<br>
If I recall correctly, there's an option to allow OMP SIMD pragmas<br>
without enabling full OMP, so that we can use it without needing all<br>
the headers and libraries, just to control vectorisation.<br>
<br>
Creating new pragmas should be seen with extreme prejudice, as these<br>
things tend to simplify the life of the compiler developers but create<br>
nightmares for application developers, especially if they want to use<br>
multiple compilers.<br>
<br>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>The section on the compatibility with OpenMP explain how -fopenmp-[simd] and -fveclib interacts. </div>
<br>
<blockquote type="cite"><br>
<blockquote type="cite" class="">2.  `#pragma omp declare variant`, same as `#pragma omp declare variant`<br>
   restricted to the `simd` context selector, from OpenMP 5.0+.<br>
</blockquote>
<br>
Is this just for the user-driven stuff? If so, let's look at it later.<br>
<br>
<br>
</blockquote>
<div><br>
</div>
<div>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). </div>
<br>
<blockquote type="cite">
<blockquote type="cite" class="">New `math.h` header file<br>
------------------------<br>
<br>
Shipped in `<clang>/lib/Headers/math.h`, contains all the declaration of<br>
the functions available in the vector library `X`, `ifdef` guarded by<br>
the macro `__CLANG_ENABLE_LIBRARY_X`.<br>
</blockquote>
<br>
So, the compiler will have the header files and the libraries will be<br>
in charge of implementing them, to avoid linkage errors?<br>
<br>
If this is a standard ABI that multiple libraries follow, I'm in<br>
favour. If we'll end up with one (or more) header(s) per library or<br>
worse, need to update the header every time the library changes<br>
something, then I'm completely against.<br>
<br>
</blockquote>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>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!)<br>
</div>
<div><br>
</div>
<blockquote type="cite">The latter will generate the compatibility issue I mentioned in one of<br>
the reviews, where the compiler has different header files but the<br>
implementations are slightly off-base. Keeping multiple copies of<br>
those libraries in the same file system (for different users in the<br>
same clusters) is even worse.<br>
<br>
That's the kind of thing that is better left for the libraries<br>
themselves. If they have both headers and objects, keeping all<br>
together into one directory is enough.<br>
<br>
</blockquote>
<div><br>
</div>
<div>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. <br>
</div>
<br>
<blockquote type="cite"><br>
<blockquote type="cite" class="">Option behavior, and interaction with OpenMP<br>
--------------------------------------------<br>
<br>
`-fveclib=X`<br>
<br>
:   The driver transform this into<br>
   `-fparse-veclib -D__CLANG_ENABLE_LIBRARY_X=1 -lX`. This is used only<br>
   for users that want to vectorize `math.h` functions.<br>
</blockquote>
<br>
Why not just include the header when you use it, instead of include<br>
and guard for all cases?<br>
<br>
<br>
</blockquote>
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<br>
<blockquote type="cite"><br>
<br>
<blockquote type="cite" class="">`-fopenmp[-simd]`<br>
<br>
:   No vectorization happens other then for those functions that are<br>
   marked with OpenMP declare simd. The header `math.h` is loaded, but<br>
   the `veclib` decorated declarations are invisible to the compiler<br>
   instance because hidden behind the `__CLANG_ENABLE_LIBRARY_X`<br>
   macros, which are not defined.<br>
</blockquote>
<br>
"No vectorisation" you mean, no "function" vectorisation. Other<br>
vectorisation (from -O3 etc) will still happen.<br>
<br>
</blockquote>
Yes, I will fix it<br>
<blockquote type="cite"><br>
<blockquote type="cite" class="">`-fopenmp[-simd] -fveclib=X` or<br>
<br>
: Same behavior as without the `-fopenmp[-simd]` option.<br>
</blockquote>
<br>
So, fveclib will enable OMP SIMD by default? I think that's what some<br>
of the reviews (particularly on certification) were against. This is<br>
not correct.<br>
<br>
</blockquote>
No, I think you got this wrong. -fveclib itself doesn't enable any OpenMP. OpenMP is enabled only when -fopenmp[-simd] is invoked.<br>
<blockquote type="cite"><br>
The only way this can work is without including OMP dependencies when<br>
using vector libraries. If the omp-simd option does not add OMP deps<br>
(as I hinted above, there may be a way), then this is fine. But if<br>
veclib flags force OMP dependencies, than this cannot work.<br>
<br>
</blockquote>
I think I haven't been clear enough on describing this last combination. Would it be better if I replace it with the following?</div>
<div><br>
</div>
<div>
<div>``-fopenmp[-simd] -fveclib=X`` or ``-fopenmp[-simd]<br>
-fveclib-include=path/to/user/provided/header/file.h``<br>
<br>
:   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.</div>
<div><br>
</div>
<div>Francesco<br>
<br>
</div>
<br>
</div>
</div>
</div>
</div>
</body>
</html>