jdoerfert added a comment. In D60583#1529937 <https://reviews.llvm.org/D60583#1529937>, @fpetrogalli wrote:
> In D60583#1529878 <https://reviews.llvm.org/D60583#1529878>, @jdoerfert wrote: > > > Why/Where did we decide to clobber the attribute list with "non-existent > > function names"? > > > The existence of those symbols is guaranteed by the "contract" stipulated via > the Vector Function ABI. They cannot be added explicitly by the front-end as > `define`s because they would be removed before reaching the vectorizer. That is not a good argument. Afaik, there are multiple ways designed to keep symbols alive, e.g., `@llvm.used`. >> I don't think an attribute list like this: >> `attributes #1 = { "_ZGVsM2v_foo" "_ZGVsM32v_foo" "_ZGVsM4v_foo" >> "_ZGVsM6v_foo" "_ZGVsM8v_foo" "_ZGVsMxv_foo" ... ` >> is helpful in any way, e.g., this would require us to search through all >> attributes and interpret them one by one. > > Agree. This is what was agreed : > http://lists.llvm.org/pipermail/cfe-dev/2016-March/047732.html > > The new RFC will get rid of this list of string attributes. It will become > something like: > > attribute #0 = { > declare-variant="comma,separated,list,of,vector,function,ABI,mangled,names" }. > > > > >> This seems to me like an ad-hoc implementation of the RFC that is currently >> discussed but committed before the discussion is finished. > > I can assure you that's not the case. > > The code in this patch is what it is because it is based on previous > (accepted) RFC originally proposed by other people and used by VecClone: > https://reviews.llvm.org/D22792 > > As you can see in the unit tests of the VecClone pass, the variant attribute > is added as follows: > > attributes #0 = { nounwind uwtable > "vector-variants"="_ZGVbM4_foo1,_ZGVbN4_foo1,_ZGVcM8_foo1,_ZGVcN8_foo1,_ZGVdM8_foo1,_ZGVdN8_foo1,_ZGVeM16_foo1,_ZGVeN16_foo1" > .... } I get that it was discussed three years ago and I get that it was accepted then. My confusing stems from the fact that it was committed just now, three years later, but shortly before the new RFC basically proposed a different encoding. > Nothing in LLVM is using those attributes at the moment, that might be the > reason why the string attribute have not yet been moved to a single attribute. That means we can easily change the encoding, and I strongly believe we should. Given that you want a different encoding, I don't know if I have to list reasons why I dislike this one (direct names as attributes). Though, I can do so if we want to discuss it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60583/new/ https://reviews.llvm.org/D60583 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
