[PATCH][ARM] PR target/66731 Fix vnmul insn with -frounding-math

[Kyrill Tkachov]

On 28/07/15 11:21, Szabolcs Nagy wrote:
> On 24/07/15 12:27, Kyrill Tkachov wrote:
>> On 24/07/15 12:10, Szabolcs Nagy wrote:
>>> (-a)*b should not be compiled to vnmul a,b with -frounding-math.
>>> Added a new -(a*b) pattern for vnmul and the old one is only
>>> used if !flag_rounding_math.  Updated the costs too.
>>>
>>> This is the ARM version of
>>> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00300.html
>>>
>>> Tested with arm-none-linux-gnueabihf cross compiler.
>>> is this OK?
>>>
>>> gcc/Changelog:
>>>
>>> 2015-07-20  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>>
>>> 	PR target/66731
>>> 	* config/arm/arm.md (muldf3negdf_vfp): Handle -frounding-math.
>>> 	(mulsf3negsf_vfp): Likewise.
>> This entry is misleading. You disable two existing patterns
>> for flag_rounding_math and you add two new patterns. The
>> entry should reflect that.
> ..
>> Can you give the new pattern a different name to reflect that
>> the neg is on the outside? Something like *negmulsf3_vfp.
> ..
>> +/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=hard" } */
>>
>> Can you please add an explicit -fno-rounding-math here? That way we get a hint as to
>> why these tests exist. Alternatively, you can rename the tests to be pr66731_1.c,
>> pr66731_2.c etc. That way in the future we'll know what issue they're testing for.
> ..
>> +float
>> +foo_s (float a, float b)
>> +{
>> +  /* { dg-final { scan-assembler "vneg\.f32" } } */
>> +  /* { dg-final { scan-assembler "vmul\.f32" } } */
>> +  return -a * b;
>> +}
>>
>> I'd prefer if you just do a scan-assembler not "vnmul", which is what this
>> patch really fixes. Whether the midend decides to use a pair of vneg+vmul
>> is tangential to this patch, it's the vnmul that we're trying to avoid.
> [v2]:
> - used different names for the new patterns
> - fixed change log accordingly
> - used explicit -fno-rounding-math in tests
> - used scan-assembler-not "vnmul"
>
> (I havent changed the names of the tests to be
> consistent with the aarch64 patches but if ppl
> prefer pr<N>.c name I can do that.)
>
> gcc/Changelog:
>
> 2015-07-28  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>
> 	PR target/66731
> 	* config/arm/arm.md (negmuldf3_vfp): Add new pattern.
> 	(negmulsf3_vfp): Likewise.
> 	(muldf3negdf_vfp): Disable for -frounding-math.
> 	(mulsf3negsf_vfp): Likewise.
> 	* config/arm/arm.c (arm_new_rtx_costs): Fix NEG cost for VNMUL,
> 	fix MULT cost with -frounding-math.
>
> gcc/testsuite/Changelog:
>
> 2015-07-28  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>
> 	PR target/66731
> 	* gcc.target/arm/vnmul-1.c: New.
> 	* gcc.target/arm/vnmul-2.c: New.
> 	* gcc.target/arm/vnmul-3.c: New.
> 	* gcc.target/arm/vnmul-4.c: New.

This is ok, thanks.

Kyrill