[Bug target/54680] New: [SH] Unnecessary int-float-int conversion of fsca fixed point input

[Bug target/54680] New: [SH] Unnecessary int-float-int conversion of fsca fixed point input

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54680

             Bug #: 54680
           Summary: [SH] Unnecessary int-float-int conversion of fsca
                    fixed point input
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
        AssignedTo: olegendo@gcc.gnu.org
        ReportedBy: olegendo@gcc.gnu.org
            Target: sh4*-*-*


SH4's fsca insn works on 16.16 fixed point inputs.  When a SFmode input is used
it must be first scaled, which is done by multiplying the input with a
constant.  It is also possible to use fixed point inputs directly to the
standard sinf / cosf functions, to avoid the scaling.

#include <math.h>
const float pi = 3.14159265359f;

float test02 (int x)
{
  return sinf (x * (2*pi) / 65536);
}

This will result in:

        lds     r4,fpul    ! 32    movsi_ie/19
        float   fpul,fr2   ! 10    floatsisf2_i4
        ftrc    fr2,fpul   ! 11    fix_truncsfsi2_i4
        fsca    fpul,dr2   ! 12    fsca
        rts                ! 35    *return_i
        fmov    fr3,fr0       ! 14    movsf_ie/1

... which is already good, but the int -> float -> int conversions can be
eliminated.
The fsca pattern is actually already prepared to support this case.  Looking at
the combine log it seems this issue could be resolved by making the fpul input
a bit more flexible:

Failed to match this instruction:
(parallel [
        (set (reg:V2SF 172)
            (vec_concat:V2SF (unspec:SF [
                        (mult:SF (float:SF (fix:SI (float:SF (reg/v:SI 164 [ x
]))))
                            (const_double:SF
9.58737992428525700000000000000000000000000000002e-5
[0x0.c90fdaa22168be2882b78df4b0b1803b8a353e85p-13]))
                    ] 16)
                (unspec:SF [
                        (mult:SF (float:SF (fix:SI (float:SF (reg/v:SI 164 [ x
]))))
                            (const_double:SF
9.58737992428525700000000000000000000000000000002e-5
[0x0.c90fdaa22168be2882b78df4b0b1803b8a353e85p-13]))
                    ] 14)))
        (use (reg/v:PSI 151 ))
    ])


Another scenario that does not work however is:

float test03 (int x)
{
  return sinf ( x * 2 * pi / 65536 );
}

(Notice the missing ( ) around 2 * pi).

It seems this is caused by the fact that the fsca pattern checks for a valid
scaling constant by doing:

   && operands[2] == sh_fsca_int2sf ()"

.. instead of looking at the values of the const_double rtx.

[Bug target/54680] [SH] Unnecessary int-float-int conversion of fsca fixed point input

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54680

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-09-23
     Ever Confirmed|0                           |1

[Bug target/54680] [SH] Unnecessary int-float-int conversion of fsca fixed point input

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54680

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-09 19:57:35 UTC ---
(In reply to comment #0)
> 
> Another scenario that does not work however is:
> 
> float test03 (int x)
> {
>   return sinf ( x * 2 * pi / 65536 );
> }
> 
> (Notice the missing ( ) around 2 * pi).
> 
> It seems this is caused by the fact that the fsca pattern checks for a valid
> scaling constant by doing:
> 
>    && operands[2] == sh_fsca_int2sf ()"
> 
> .. instead of looking at the values of the const_double rtx.

No, that's not the reason.  The reason is that in the above case 'x * 2' is
expanded as an integer multiplication.  Thus the fsca scale factor changes and
is not pi / 2**15 as expected by the fsca pattern.
This could be fixed by inspecting the fpul operand in detail, but it doesn't
seem worth doing so.  Anyway user code that will use the standard sinf / cosf
functions with fixed-point inputs will be aware of SH4 fsca specifics.

[Bug target/54680] [SH] Unnecessary int-float-int conversion of fsca fixed point input

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54680

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-12 23:19:32 UTC ---
Author: olegendo
Date: Fri Oct 12 23:19:27 2012
New Revision: 192416

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=192416
Log:
    PR target/54680
    * config/sh/sh.c (sh_fsca_sf2int, sh_fsca_int2sf): Fix swapped
    comments.
    * config/sh/predicates.md (fpul_operand): Add comment.
    (fpul_fsca_operand, fsca_scale_factor): New predicates.
    * config/sh/sh.md (fsca): Move below sincossf3 expander.  Convert to
    insn_and_split.  Use fpul_fsca_operand and fsca_scale_factor predicates.
    Simplify fpul operand in splitter.

    PR target/54680
    * gcc.target/sh/pr54680.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr54680.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/54680] [SH] Unnecessary int-float-int conversion of fsca fixed point input

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54680

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-30 01:28:16 UTC ---
Fixed in 4.8.