Prazek added a comment. In https://reviews.llvm.org/D32401#738513, @rjmccall wrote:
> In https://reviews.llvm.org/D32401#735127, @Prazek wrote: > > > In https://reviews.llvm.org/D32401#734921, @rjmccall wrote: > > > > > I continue to be really uncomfortable with the entire design of this > > > optimization, which appears to miscompile code by default, but as long as > > > nobody's suggesting that we actually turn it on by default, I guess it > > > can be your little research-compiler playground. It might be better to > > > downgrade it to a -cc1 option, though. > > > > > > This specific change is fine by me. > > > > > > Can you tell me a little more about what part of the design you dislike? Is > > it about missing optimizations by introducing the barriers, cost of > > inserting the barriers or the fact that we have to be cautious to not break > > anything? > > > The latter. The optimization design seems to rely on anticipating every case > that should disable the optimization, hence this patch adding special-case > logic to the frontend, and the 3 other patches you've got out for review > adding special-case logic to different parts of the frontend, all on top of a > ton of special-case logic in yet more parts of the frontend from when you > implemented the optimization in the first place. There is an additive > problem here where suddenly the design of this specific optimizaton becomes > an affirmative burden to basically all the code in the frontend and, > presumably, the middle-end and beyond, as opposed to just defaulting to > correct behavior. There is zero chance that this latest collection of > changes is actually fixing all of the problems; it's just papering over the > next round of testing. > > I'm very sympathetic, because I know this is an important optimization, but > it's not clear to me that it's actually reasonable to implement in LLVM. > > John. We will probably not gonna make it in the first time, but the issues I am solving are known almost from the beginning - we knew that there is an issue with comparing pointers, and the idea of putting barriers for comparision was showed on last summer. Adding barriers for pointer casts is the consequence of that - either we will add barriers for comparisions of all pointers , or only to the ones having any vptrs + add barriers when we loose the dynamic information. The issues with linking is also known, I just solve some small details that we missed :) I hope I will be able to show some statistics soon on how devirtualization is doing to show that it is worth the cost :) https://reviews.llvm.org/D32401 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
