[PATCH] D27250: [OpenMP] TargetLibraryInfo from "declare simd".

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In https://reviews.llvm.org/D27250#609111, @fpetrogalli wrote:

> In https://reviews.llvm.org/D27250#609006, @ABataev wrote:
>
> >
>
>
> Hi! Thanks for your review!
>
> > 1. Please provide full context for your changes (check this document how to do this http://llvm.org/docs/Phabricator.html).
>
> I missed that out, should be done now.
>
> > 2. Use clang-format for all your changes.
>
> Done.
>
> > 3. Do not use backed infrastructure to mangle the name of the new function. Instead, you should build clang-based declaration and use clang infrastructure to ge
>
> The second part of the sentence is missing. I guess you are saying I should use the infrastructure for name mangling in clang in llvm? Is that what you mean? I think that the right place to keep something that need to be shared between LLVM and clang is the LLVM codebase, not conversely. Can we wait for other people feedback before deciding here?


Yes, that's what I meant. Clang supports both Microsoft and Itanium-based manglings and in clang you should use clang-based solution.

> 
> 
>> Also, I don't like the idea of the generation of real function profiles in the frontend. It was decided, that the frontend only provides a list of mangled names for vector functions, which must be generated in the backend.
> 
> I am just generating  a set of global variable with external linkage that are deleted by LLVM before any code generation happens. The globals are used just to populate the TargetLibraryInfoImpl class instance created in BackendUtils.cpp. See the high level description of the functionality I have provided in the RFC mail (see [1] below).I see this mechanisms being easier rather than getting a name, de-mangle it, interpret the parameters section, and then generate whatever information the vectorizer need to vectorize a particular scalar function in a loop. Instead of introducing new structures for matching what the vectorizer need swith what the TLI provides,  I just use what llvm already has, the FunctionType class.

It does not matter. You should discuss it before trying to implement, especially if another solution was already accepted and implemented. Xinmin already answered you, please sync with him.

> This also avoid the complications of having to write a demangle function for the vector names for each architecture, which might have completely different names across architectures and ABI versions. With this FunctionType mechanisms, ABI changes are transparent to vectorizer.
> 
>> But you're trying something different. If you're going to change the whole solution you should send an RFC to the community
> 
> I've sent out an email, apologies for not specifying RFC in the subject. You can find a copy of it with the RFC in the mailing list now [1].
> 
> [1] http://lists.llvm.org/pipermail/llvm-dev/2016-November/107677.html




https://reviews.llvm.org/D27250