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

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

In D70107#2019000 <https://reviews.llvm.org/D70107#2019000>, @fpetrogalli wrote:

> In D70107#2018811 <https://reviews.llvm.org/D70107#2018811>, @anna wrote:
>
> > 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.


Thanks for the clarification here.

> 
> 
>> 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 agree with all of the points. Again, to state, for a simple scalar function, we will have 5 vector forms being generated (2,4,8,16 and 32) and we'll have to start recording each of those declarations in the module. Is that right? This will functionally work for us (since we've tried a similar idea in our pipeline).

> 
> 
>> 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! :)

Ah, so there is some difference here. Our front-end and LLVM is completely decoupled (more details here: https://llvm.org/devmtg/2017-10/slides/Reames-FalconKeynote.pdf), but we have a mechanism to query from LLVM to our java VM for anything we want more information about (in this case, pass in the exact set of declarations). So, we can always guarantee the correct set of declarations are retrieved. Building the signature at compile time without any input from FE will be problematic (as you have pointed out above). I can see why the declarations are marked as required for the attribute.

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

Agreed, I was just it  pointing out (and to be clear, this seems to be the assumption in various other parts of the vectorizer as well). :)

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

So, as stated previously in numbers, we have something like 5 * number of scalar functions which have vector mappings. In our case, we will have 5 per scalar because there is nothing preventing generating a vectorized power-of-2 VF. I remember seeing some "vector length agnostic function", perhaps those can be generated on the fly, if we specify something like _vN rather than _v2 or _v4 etc?

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70107