[PATCH] IndVarSimplify does not check if loop invariant expansion can trap

[Rafael Ávila De Espíndola]

We are more likely to fix a crash. So I don't think it is firing. The only case this could regress is a potentially trapping operation that doesn't actually trap for reasons that are invisible to the optimizer.

Adding the extra control flow to guard it is not clear to be a win. We should probably fix the miscompilation first. If we do have a perf regression somewhere, it can be addressed in a subsequent patch.

Sent from my iPhone

On 2013-06-04, at 11:22, James Molloy <james at jamesmolloy.co.uk> wrote:

> Hi Rafael,
> 
> I don't know. Do we know this broken optimization doesn't kick in in the test suite? If it does, we'd regress on performance if we took the conservative way out.
> 
> Just my 2c.
> 
> Cheers,
> 
> James
> 
> On 4 June 2013 15:40, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> On 4 June 2013 09:49, James Molloy <james at jamesmolloy.co.uk> wrote:
>> > Hi David,
>> >
>> > Might a more performant solution not be to wrap any hoisted instructions
>> > that are not safe to expand in a conditional that tests if the loop will be
>> > entered at least once?
>> 
>> Is that really worth it? Given that this bug went undetected for years
>> (it was already broken in r122610), I don't think it would kick often
>> enough to justify it.
>> 
>> Cheers,
>> Rafael
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130604/52350256/attachment.html>