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

[David Majnemer]

Committed in r183239.


On Tue, Jun 4, 2013 at 8:48 AM, James Molloy <james at jamesmolloy.co.uk>wrote:

> > The only case this could regress is a potentially trapping operation
> that doesn't actually trap for reasons that are invisible to the optimizer.
>
> Like a divide operation where the divisor never happens to be zero? or am
> I misunderstanding the patch?
>
> My understanding is that now we will not hoist divides out of induction
> variable expressions where before we did, because divides can trap. Most
> divides in user code don't trap, so I don't see how you can conclude that
> the optimization is not firing on user code.
>
> That said, I agree let's fix the crash with the trivial fix proposed and
> deal with the performance impact later.
>
> On 4 June 2013 16:41, Rafael Ávila De Espíndola <
> rafael.espindola at gmail.com> wrote:
>
>> 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/2433eb8a/attachment.html>