PATCH: do ?: lowering to selects less wrong

[Nick Lewycky]

On 5 November 2013 17:44, Nick Lewycky <nlewycky at google.com> wrote:

> On 5 November 2013 17:30, Rafael Espíndola <rafael.espindola at gmail.com>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.

Nick


> On 5 November 2013 16:54, Nick Lewycky <nlewycky at google.com> 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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131108/600546da/attachment.html>