[PATCH] D61604: [InstCombine] sink FP negation of operands through select

[Sanjay Patel via llvm-commits]

If we change the IRBuilder, that's not going to catch cases where a
regression test already has the 'fsub -0.0', is it? I'd think that only
comes into play if the regression test creates that op as part of its
transformations.

On Tue, May 7, 2019 at 12:20 PM Cameron McInally <cameron.mcinally at nyu.edu>
wrote:

> On Tue, May 7, 2019 at 11:44 AM Sanjay Patel <spatel at rotateright.com>
> wrote:
>
>>
>> On Tue, May 7, 2019 at 7:16 AM Kevin Neal <Kevin.Neal at sas.com> wrote:
>>
>>>
>>> How should we handle regressions in code quality that are exposed by
>>> tests?
>>>
>>
>> One idea I had (or I may be remembering someone else's suggestion) was to
>> grep the existing IR regression tests for 'fsub -0.0', then create a
>> sibling test using fneg. If that doesn't result in the same output, then we
>> need to do some pattern matching enhancement.
>>
>
> I'm not opposed to this, but it does sound like a ton of work.
>
> What if we checked for a -0.0 constant in IRBuilder::FSub*(...) and
> created an FNeg instead? Then we should only see test changes with `fsub
> -0.0`-> `fneg`. If we find anything else, it would need a closer look. Does
> that miss anything?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190507/54917cf8/attachment.html>