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

[Cameron McInally via llvm-commits]

Ah, yeah. That was wrong. I should have said
BinaryOperator::CreateFSub*(...).



On Tue, May 7, 2019 at 3:07 PM Sanjay Patel <spatel at rotateright.com> wrote:

> 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/b10c15a0/attachment.html>