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

Michel Dänzer michel at daenzer.net
Wed Aug 6 00:25:53 PDT 2014


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?


> 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



More information about the llvm-commits mailing list