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

Mon May 4 14:32:25 PDT 2020

anna added a comment.

Thank you for the detailed and quick response @fpetrogalli.

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.

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).

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? 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 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).

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), 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), we will start having way too many declarations in module (and the pass will also need to be updated). 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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70107