fpetrogalli added a comment.

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?

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

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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to