[PATCH] R600/SI: Use source modifiers for f64 fabs / fneg

[Matt Arsenault]

On Aug 6, 2014, at 12:25 AM, Michel Dänzer <michel at daenzer.net> wrote:

> On 06.08.2014 15:34, Matt Arsenault wrote:
>> 
>> On Aug 4, 2014, at 1:09 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> 
>>> On 02.08.2014 11:33, Matt Arsenault wrote:
>>>> f32 has a comment and patterns about the behavior with 0 and negate
>>>> modifier which I'm unsure about. I think the source modifier should
>>>> work if the source is 0, since the negate modifier is supposed to be
>>>> applied before abs. Does this pattern need to be skipped for f64
>>>> too?
>>> 
>>> The original problem was that fneg was implemented as V_ADD_F32_e64 of 0
>>> and the operand with the negate source modifier set. This resulted in
>>> +0.0 instead of -0.0 (i.e. the sign bit was not set) for an argument of
>>> +0.0. There's a (graphics) piglit test which failed because of that.
>>> 
>>> Given that FNEG_SI and FABS_SI directly manipulate the sign bit, I
>>> wouldn't expect that to require any special treatment anymore. As you
>>> say, the source modifiers should work fine when folding these into other
>>> operations.
>>> 
>> 
>> 
>>> So, I suspect the purpose of the (fneg (fabs)) pattern isn't to avoid
>>> that problem but to avoid expanding both FABS_SI and FABS_SI.
>> 
>> 
>> Yes, that seems to be what it accomplishes.
> 
> A solution preserving that but allowing the (fneg (fabs)) to be folded
> into other instructions might be to add FNEG_FABS_SI?

This already works correctly. The complex patterns check for both the ISD::FNEG and ISD::FABS and fold them.


> 
> 
>> f64 fneg is still implemented that way because there is no v_sub_f64.
> 
> I suspect v_sub_f* would have the same issue as v_add_f32 did anyway.
> 
> 
>> Here is an updated set that implements that for f64. These also
>> switch to preferring using the literal constant as the first operand,
>> rather than loading the immediate into a register.
> 
> Sounds good, but I have to defer to Tom or others for review.
> 
> 
> -- 
> Earthling Michel Dänzer            |                  http://www.amd.com
> Libre software enthusiast          |                Mesa and X developer

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140806/f06e033c/attachment.html>