[llvm-dev] [cfe-dev] [RFC] Expose user provided vector function for auto-vectorization.

[Francesco Petrogalli via llvm-dev]

> On Jun 4, 2019, at 12:23 PM, Doerfert, Johannes <jdoerfert at anl.gov> wrote:
> 
> Hi Francesco,
> 
> On 06/03, Francesco Petrogalli wrote:
>>> On Jun 3, 2019, at 1:43 PM, Andrea Bocci <andrea.bocci at cern.ch> wrote:
>>> as a candidate future user of the proposed extension, I think I like the simplified proposal better than the original RFC.
>>> 
>>> The only part of the syntax that I would find not very much user-friendly is having to mangle the isa/mask/vlen/type list by hand.
>>> 
>>> Would a more C-like syntax be feasible ?
>> 
>> The syntax I proposed allows user to expose concepts like “linear”, “uniform” and “aligned" parameters, as described in the `declare simd` directive of OpenMP, which is what x86 and aarch64 Vector Function ABIs use to mangle the names.
>> 
>> I agree with you that having to manually specify the mangling is a bit cumbersome, I am open to suggestions.
>> 
>> An alternative approach was to make the actual string inside the attribute to accept anything a `declare variant` in a `simd` context could accept, but that wold probably raise the same problems that `declare variant` has raised.
>> 
>> In fact, on AArch64, a typical use of declare variant would be more complicated than just specifying `simd` in the `context`, with `simdlen` and `[not]inbranch`. It would require also to specify the `isa` trait from the `device` set, and to use a `vendor` specific set to be able to list scalable (i.e. vector-length agnostic) functions for SVE.
>> 
>> This would definitely look like implementing `declare variant`, as the attribute would need to accept something like the following:
>> 
>> ```
>> . . . __attribute__(simd_variant(“vector_version”,“context={simd(simdlen(2),notinbranch”},device={isa(“simd")})) 
>> ```
>> 
>> Using the sub-string from the mangled name to me has the following advantages:
>> 
>> 1. No need to deal with `declare variant`.
>> 2. [...]
>> 3. In terms of usability, there is no need to reinvent the wheel. The
>> `declare variant` of OpenMP 5.0 is perfectly suited for what we are
>> doing here, it is just much more than what it is needed for exposing
>> user-defined function to the vectorizer. So, on the long term, I think
>> we should definitely implement `declare variant`. Whatever attribute
>> system we came up with this RFC, it will be easy to map the `simd`
>> bits of `declare variant` to it.
> 
> I'm unsure about this. While I agree that avoiding to deal with "declare
> variant" will make this all much easier, you seem to agree that we will
> eventually have to do that anyway.

Please notice that by “avoiding to deal with `declare variant`” doesn’t mean that I am not defining the attribute with the OpenMP directive `declare variant` in mind. It just means that I don’t want (for now, int his RFC), discuss how to implement _full_ support for `declare variant` for the feature discussed in this RFC.

> Adding something now that is a subset
> of what we will need will put as in an awkward position.

Agree, and that’s why we need to craft it carefully to make sure we don’t get to that point. Which is something that owe are definitely doing.

> We will have to
> unify the existing in-house concept with the standardized OpenMP way in
> terms of semantics as well as implementation.

Yes.

> I somehow doubt that this
> will make things easier, especially if we later discover that the
> in-house solution is somehow incompatible or not actually a subset.
> 

Agreed. The semantic of `declare variant` has more meanings than the `declare simd` itself. But the specs state that the `simd(other-stuff)` trait that can be passed to the `construct` set mirrors the semantic of the `declare simd` directive.

“The simd trait can be further defined with properties that match the clauses accepted by the declare simddirective with the same name and semantics.” (Page 52, lines 9-10).

The Vector Function ABI mangling scheme represent all the information that can be passed via a `declare  simd` directive (more than that, because it encodes also the target vector extension in the <isa> token). I can definitely speak for the AArch64 one, but it seems the same method has been used for the x86 one (@Hideki, @Xinmin, can you confirm?).

Therefore, we can all agree that the attribute `vector-variant`, as it has been proposed, can be used to represent `declare simd` and `declare variant`, the latter when used with the `simd` trait of the `construct` set.

The question that you raise is: “can we extend the information carried by `vector-variant` to represent all the semantic of `declare variant`?“

Even if the answer to such question is no, is it really what we want to do?

I am just thinking at a situation where the `declare variant` is used not for a single trait like `simd`, but also for other contexts.

```
#pragma omp declare variant(variant_foo) match(construct={simd(simdlen(2),notinbranch), another_trait(with_other_clause)}, device= {isa(TARGET_ISA)}, ...)
double foo(double);

Vector_double variant_foo(vector_double);
```

The pragma here is giving the following information:

1. The function `variant_foo` is obtained by `foo` with the following instance of `declare simd`:

#pragma omp declare simd simdlen(2) notinbranch
double foo(double);

2. The function `variant_foo` is written using the instructions of the TARGET_ISA specified in the `isa` trait of the `device` set.

Again, (I will never get tired stating that) the Vector Function ABI name mangling scheme suffices to represent information 1. and 2.

 3. The directive is saying something more, as it lists `another_trait` in the `construct` set, or might be listing an `extension` set with `vendor` specific rules.

All this addition information doesn’t invalidate what is encoded in 1. And 2.  

This means that to represent `declare variant` information we don’t have to worry about _extending_ the `vector-variant` attribute, but that we just need to add additional attributes attached to the same `variant_foo` that describe the additional context information expressed in the `declare variant`.

So, at the end, once we have full support for `declare variant`, the IR attribute that we will associate to `foo` will look like:

```
. . . = call double @foo(double . . .) #0

attribute #0 = {vector-variant=“_ZGVwhatever_foo”, vendor-variant=“X”, extension-variant=“Y”, another_trait_variant=“A,B,C”}

```

It will be then a matter of interpreting the data in attribute #0 to decide whether or not it is appropriate/convenient to use `variant_foo` instead of `foo` in whatever optimization is being performed.

As a last comment. Maybe the name `vector-variant` for the attribute is to close to use of `variant` in the `declare variant` directive.

If that is something that have helped raising the idea that with my proposal I was trying to use `vector-variant` to implement `declare variant`, I am happy to change it to something that is specific to the `declare simd` context, something like `declare-simd = “_ZGVn. . . , _ZGVs. . .”`. Lack of fantasy here, but I must say that after thinking about the information it represent, I am kinda incline in preferring it over `vector-variant`. This shouldn’t be a problem for the VecCLone pass because the pass it not yet in master and therefore there is no prior IR that uses `vector-variant` in production.

> To summarize: I will not oppose this effort but I would much rather see
> us working towards "declare variant”,

Hopefully this last message is a step in that direction.

> a stronger concept we will have to
> support in the near future anyway.
> 
> Thanks,
>  Johannes

You are very welcome, and thank you for forcing me thinking more about this aspect!

Kind regards,

Francesco