[llvm-commits] [PATCH] PowerPC: More support for Altivec compare operations

[Adhemerval Zanella]

Thanks for the comments Alex:


On 10/24/2012 09:23 PM, Alex Rosenberg wrote:
> On Oct 24, 2012, at 7:49 AM, Adhemerval Zanella wrote:
>
>> +    // The VSET0 contains all bits sets to '0' while VMASK all '1'. They will be
>> +    // used later on a 'vsel' instruction to select the result from the compare
>> +    // instructions.
>> +    SDValue VSET0(CurDAG->getMachineNode(PPC::V_SET0, dl, VecVT), 0);
>> +    SDValue VMASK(CurDAG->getMachineNode(VSPLATInst, dl, VecVT, getI32Imm(-1)), 0);
> The vcmpXXX instructions already emit lane masks. At most, one of these requires inverting the result. Materializing a 0 and a -1 value and using a vsel seems unnecessary.

You are right, I overlooked it in my first attempt and removed the logic.


>> -def : Pat<(fmul VRRC:$vA, VRRC:$vB),
>> +def : Pat<(v4f32 (fmul VRRC:$vA, VRRC:$vB)),
>>            (VMADDFP VRRC:$vA, VRRC:$vB, (v4i32 (V_SET0)))>; 
> This is incorrect for an input of -0.0. You should add -0.0 to produce the correct results, at least without -ffast-math or similar choice. -0.0 can be materialized with a vslw of -1 by -1.

I removed this snippet since your observations are correct and this correction also
does not really relates to the patch main corrections (altivec compares). I'll work
on this in a different patch.

I cleanup the patch now and I believe it OK. Any thoughts, comments, advices?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-PowerPC-More-support-for-Altivec-compare-operations.patch
Type: text/x-patch
Size: 21153 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121029/5c145837/attachment.bin>