[PATCH] D70107: [VFABI] TargetLibraryInfo mappings in IR.

Mon May 4 16:09:26 PDT 2020

fpetrogalli added a subscriber: aranisumedh.
fpetrogalli added a comment.

In D70107#2018811 <https://reviews.llvm.org/D70107#2018811>, @anna wrote:

> Thank you for the detailed and quick response @fpetrogalli.

You are welcome!

> In D70107#2018558 <https://reviews.llvm.org/D70107#2018558>, @fpetrogalli wrote:
> 
>> In D70107#2018145 <https://reviews.llvm.org/D70107#2018145>, @anna wrote:
>>
>> > @fpetrogalli  I have a high level question - why does this pass require that the function should be from one of the vector libraries (Mass, SMVL etc)? I'd like to piggy-back on the `vector-function-abi-variant` attribute to vectorize a call to a scalar function, once the front-end adds this attribute to the callsite. So, if we have the following code in IR, with a scalar call to a function `trivially.vectorizable.func` with the `vector-function-abi-variant` attribute, we will generate the vector function declarations and then LoopVectorizer/SLPVectorizer would do its thing.
>>
>>
>> Hi @anna ,
>>
>> this is exactly what the attribute is for. This pass works only with the library recognized by the TLI as a shortcut implementation to avoid doing the codegeneration of the attribute for the library functions in the frontend.
>>
>> As things are, if you run `opt` with `-O2` on your IR, the code should be vectorized with a call to the vector version, if  not for the fact that the IR is missing the declaration of the actual vector function. In fact, the vector-function-abi-variant attribute requires that all the mappings listed in the attribute are resolved by some declaration/definition in the IR.
> 
> 
> Ah, interesting. I hadn't noticed that property of the attribute. So, basically, if we were to pass the attribute through the front-end, we're required to keep the vector declarations in the IR as well.

Yep! I have added as many assertions as I could to make sure the declaration was not missing when reading the attribute.

> For my purposes, our front-end converts the code into IR and our use case works with adding the attribute at that point itself. I was hoping to actually avoid having the various vector declarations in the IR - frankly it's just for IR cleanliness and some compile time (depending on number of vector variants for each such scalar function) since the vector declarations won't typically be used until we do a pass for some form of vectorization (loop/SLP etc).

I understand - this was our original intent too. Add the attribute and build the vector function signature at compile time in the middle end, our of the scalar signature in the IR and the mangled string of the vector function name. We started this work last summer, until our intern @aranisumedh found out that it was not possible to retrieve the vector signatures in some cases [1]. Essentially, it turned out that only the frontend has enough information about the language types that are needed to build the correct signature in IR. hence, we decided the vector declaration to be required in the IR.

[1] See this: http://lists.llvm.org/pipermail/llvm-dev/2019-June/133225.html, and the follow up discussion. Let me know if you have any question!

> what I was thinking of adding in `inject-TLI-mappings` or a separate pass such as "inject-vector-declarations"  is:
> 
>   if (call->hasAttr("variant-abi")) {
>     getVectorVariantFromAttr(call, VariantNames)
>     for (VName: VariantNames) 
>       addVariantDeclaration(CI, VF, VName);
>   }
> 
> 
> So, to that end, I think it would be better to make the  property that the "vector name mentioned in the attribute should be present in the IR as a vector function declaration" as optional?

Sorry, I don't think we can do this, for the aforementioned reason, and for the sake of consistency. It would be bad to end up having code to handle the custom behavior of the attribute.

> This would also go better with the work of converting "pragma simd" to vector-abi attribute, since the front-end needn't add all the various vector declarations (until actually it is required by some pass, such as loop/SLP vectorizer). Again, this is just for IR compactness, compile time benefits. We do not have any functional issues with the attribute as-is, and we do not need source language support for pragma-simd (more details below).
> 
>> Yes, this is correct. Your code will vectorize as it sees the vector declaration. Just make sure to mark it with @llvm.compiler.used so it doesn't get deleted by other optimization in the pipelines before reaching the vectorizer.
> 
> Yup, the main thing was I wanted to avoid having the declarations in the IR, but I see that it is the property of the attribute itself.
> 
>> Changing the codegeneration of declare simd to use vector-function-abi-variant is on my to do list, unfortunately not as close as I would like them to be. I suspect that also you have your own pipeline of things to do, but let me know if declare simd becomes closer to you than to me, I can always help you getting things sorted!
> 
> For us, we are able to pass in the "vector-function-abi-variant" since the function we're tring to vectorize is not from the source code (java), so that part is sorted. We are adding these vectorized versions of some internal functions we add through the front-end (not present in source code).

I think your starting point should be to make sure that the front end generates the exact list of functions you want to provide in vector form, using the attribute and relative declarations. Once you have verified the declarations are there, check that the vectorizer vectorizes as expected. If not, improve whatever part of the middle end opt that is needed to make your input IR work.

> I was just curious if we had some other attribute I hadn't noticed :)  It's just that adding these bunches of declaration from front-end seemed a lot of such declarations going through each pass (number of scalar functions * 5).

I can see that this might not "look nice" from some points of view, but it is the best way to guarantee that front-end and middle-end are decoupled to be able to unit-test each components independently. In my past I have gone through testing a front-end coupled with a backend - you don't wanna do that if you want to keep sane! :)

> Also, this is slightly OT: the pass seems to rely on the fact that vectorizer will always use power-of-2 VF (which is true currently),

the pass assumes power of 2 because the TLI assumes power of two. The pass doesn't know anything about the vectorizer.

> but once we start supporting any number for VF (for example in middle-end it's VF=6 and backend decides what's the correct VF is),

Of course, the TLI assumes power of 2 because the vectorizer assumes power of 2. It is a chain. If you want to vectorize VF=6, I think you should start from the vectorizer.

>   we will start having way too many declarations in module (and the pass will also need to be updated).

I think you need to define how many are too many. Even if the IR file will seem to have many unused declarations, those will not end up in an object file, and will not be useless because they could be used by other optimization passes if needed. It seems to be the only way we can keep the scalar-to-mapping info in a useful place.

> I think we're functionally good in that case, because we will just not generate the vectorized call, since we don't have the corresponding VF variant declaration.

Yep. The `VFDatabase` is the interface you want to use to check the availability of vector functions in the IR, for a given `CallInst` CI (please refer to the code in the vectorizer, and the implementation of `VFDatabase` and its API in `llvm/Transforms/VectorUtils.h`).

Sorry if some of my answer sound "on the go", these days I am working odd hours and have many things in the pipeline! :)

Please don't stop asking questions!

Thank you!

Francesco

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70107/new/

https://reviews.llvm.org/D70107