On Fri, Nov 8, 2013 at 2:57 PM, Nick Lewycky <[email protected]> wrote:
> On 5 November 2013 17:44, Nick Lewycky <[email protected]> wrote: > >> On 5 November 2013 17:30, Rafael Espíndola <[email protected]>wrote: >> >>> The test passes even with the code change reverted. >>> >> >> Bah! Thanks for noticing. The testcase worked in C++ mode but not C mode. >> >> Adding it to test/CodeGenCXX/catch-undef-behavior.cpp doesn't work well >> either because of ordering dependencies in that test. >> >> Updated patch attached! >> > > In person at the llvm dev mtg, Richard LG'd this patch with comments. I > wanted to make sure those comments are on the mailing list. > > Just because Evaluate returns true doesn't mean that IRGen will emit a > constant. It means that it's constant evaluable and that the optimizers > could form a constant out of it. Of course, what is a constant anyways, > many llvm constants lower to multiple instructions. > > So assuming that we want to have this method at all, this is okay as an > incremental patch but it's not particularly satisfying. We'd rather have > IRGen detect whether it's an AST that would actually lower to a constant. > Yep. I'm happy for this to go in as-is, but what I think we really want here is: Try to emit the LHS and RHS as constants. If you succeed, emit a select, otherwise emit a branch to evaluate the operand(s) that we couldn't emit as constants. > Nick > > >> On 5 November 2013 16:54, Nick Lewycky <[email protected]> wrote: >>> > When lowering "cond ? X : Y" we do some safety checks to see whether >>> we can >>> > instead emit both X and Y and use a llvm select instruction to choose >>> > between them. This code is insufficiently safe, and introducing loads >>> into a >>> > program that didn't load is a bad idea. For example, it could be TLS. >>> It >>> > could be a non-volatile auto in another function that isn't the current >>> > function (think lambdas). >>> > >>> > Don't do this here. LLVM knows how to do this properly. >>> > >>> > Patch attached, please review! >>> > >>> > Nick >>> >> > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
