erichkeane added a comment. In https://reviews.llvm.org/D38596#913322, @rsmith wrote:
> The big pain point here is that we don't know whether a function is > multiversioned until we see the *second* version of it (or a declaration with > no target attribute or a declaration with a "default" target), and we > generally want our AST to be immutable-once-created. I don't see a good way > to handle this except by mutating the AST when we see the second version > (which leads to complexity, particularly around PCH -- you need to inform AST > mutation listeners that this has happened and write a record out into the AST > file to indicate you've updated an imported entity) -- or by treating any > function with a target attribute as if it were multiversioned, and not > treating a declaration with a body as being the definition until / unless we > see the first use of it (which, again, leads to an AST mutation of some kind > at that point). > > Keeping the redeclaration chains separate would avoid this problem, but we'd > still need to do *something* to track the set of versions of the function so > we can emit them as part of the ifunc. That is definitely a pain point. The solution I've had bouncing around in my head is the following, but it has some limitations that GCC doesn't have: 1- Convert to multiversion not permitted unless 1 of the 2 is 'default' (since it is required). 2- Convert to multiversion not permitted if the decl has been used. 3- Prevent the 'lookup' from finding the non-default version, this permits things like constexpr to work properly. 4- Add a list of the "Multiversion Decls" to the 'default' FunctionDecl. My hope being that #2 makes it so that no one would really NOTICE the AST rewriting, since it hasn't been used. #1 enables #3, so that the only 'user' of the multiversion'ed decls ends up being the CodeGen emitting the ifunc. #4 of course is to give CodeGen the ability to emit all of the definitions. Is there a better way to 'downgrade' the non-default FunctionDecl's to 'not lookupable'? Or is this something I would need to add? Finally, 1 further idea is that "default" automatically creates it as a 'multiversion case'. Since a sole 'default' would be invalid in GCC, I think that makes us OK, right? Thoughts on the above proposal? I have basically abandoned this review, since the changes are pretty intense. I've also submitted another patch (that Craig Topper is helping me with) to cleanup the CPU/Feature definitions to make that list more sane. BTW: Just saw your 2nd response: I think the rejection of cases where multiversioning is caused after first-use is the only real way to make this sane. Thanks again for your input! https://reviews.llvm.org/D38596 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
