rjmccall added a comment. 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. https://reviews.llvm.org/D32401 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
