[Bug target/17390] missing floating point compare optimization

[Bug target/17390] missing floating point compare optimization

[uros at kss-loka dot si]

------- Comment #4 from uros at kss-loka dot si  2005-10-19 12:29 -------
Patch here: http://gcc.gnu.org/ml/gcc-patches/2005-10/msg01126.html


-- 

uros at kss-loka dot si changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |uros at kss-loka dot si
                   |dot org                     |
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2005-
                   |                            |10/msg01126.html
             Status|NEW                         |ASSIGNED
           Keywords|                            |patch
   Last reconfirmed|2005-09-24 16:56:00         |2005-10-19 12:29:11
               date|                            |


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

[Bug target/17390] missing floating point compare optimization

[steven at gcc dot gnu dot org]

------- Comment #5 from steven at gcc dot gnu dot org  2005-10-19 13:13 -------
That patch is yet another example of why we constantly keep having compile time
problems.  Just add more, and more, and more, and more.  And act surprised when
someone notices that gcc 4.1 is four times as slow as 2.95.3 for very little
benefit on x86 at least.

Why a new pass over all insns, instead of e.g. teaching postreload cse or
postreload gcse about this kind of thing?


-- 


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

[Bug target/17390] missing floating point compare optimization

[uros at kss-loka dot si]

------- Comment #6 from uros at kss-loka dot si  2005-10-24 06:29 -------
Some discussion can be found in this thread:
http://gcc.gnu.org/ml/gcc/2005-10/msg00574.html

A new version of patch is here (post-reload machine dependant pass):
http://gcc.gnu.org/ml/gcc-patches/2005-10/msg01298.html


-- 

uros at kss-loka dot si changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://gcc.gnu.org/ml/gcc-  |http://gcc.gnu.org/ml/gcc-
                   |patches/2005-               |patches/2005-
                   |10/msg01126.html            |10/msg01298.html


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

[Bug target/17390] missing floating point compare optimization

[pinskia at gcc dot gnu dot org]

------- Comment #7 from pinskia at gcc dot gnu dot org  2006-01-18 04:57 -------
Hmm, I get (but that looks like different branch predictions):
_testf:
        fldl    4(%esp)
        ftst
        fnstsw  %ax
        testb   $64, %ah
        jne     L10
        ftst
        fnstsw  %ax
        fstp    %st(0)
        testb   $69, %ah
        jne     L5
        fld1
        ret
        .align 2,0x90
L10:
        fstp    %st(0)
        fldz
        ret
L5:
        flds    LC2
        ret


-- 


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

[Bug target/17390] missing floating point compare optimization

[uros at kss-loka dot si]

------- Comment #8 from uros at kss-loka dot si  2006-01-18 09:50 -------
(In reply to comment #7)

> Hmm, I get (but that looks like different branch predictions):

It looks that your default is -mtune=pentium.

> _testf:
>         fldl    4(%esp)
>         ftst
>         fnstsw  %ax
>         testb   $64, %ah
>         jne     L10
>         ftst
>         fnstsw  %ax
>         fstp    %st(0)
>         testb   $69, %ah
>         jne     L5
>         fld1
>         ret
>         .align 2,0x90
> L10:
>         fstp    %st(0)
>         fldz
>         ret
> L5:
>         flds    LC2
>         ret

With proposed patch, this code is compiled to (-O2 -ffast-math -mtune=pentium
-fomit-frame-pointer):

testf:
        fldl   4(%esp)
        ftst
        fnstsw %ax
        fstp   %st(0)
        testb  $64, %ah
        jne .L10
        testb  $69, %ah
        jne .L5
        fld1
        ret
        .p2align 4,,7
.L10:
        fldz
        ret
.L5:
        flds   .LC2
        ret

and for -mtune=i686:

testf:
        fldl   4(%esp)
        ftst
        fnstsw %ax
        fstp   %st(0)
        sahf
        je .L10
        jbe .L5
        fld1
        ret
        .p2align 4,,7
.L10:
        fldz
        .p2align 4,,8
        ret
.L5:
        flds   .LC2
        .p2align 4,,4
        ret

BTW: I'll attach a patch, rediffed to current SVN.


-- 


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

[Bug target/17390] missing floating point compare optimization

[uros at kss-loka dot si]

------- Comment #9 from uros at kss-loka dot si  2006-01-18 09:53 -------
Created an attachment (id=10666)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10666&action=view)
patch to SVN GCC: (GNU) 4.2.0 20060117 (experimental)


-- 


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

[Bug target/17390] missing floating point compare optimization

[pluto at agmk dot net]

------- Comment #10 from pluto at agmk dot net  2006-05-01 08:05 -------
(In reply to comment #9)
> Created an attachment (id=10666)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10666&action=view) [edit]
> patch to SVN GCC: (GNU) 4.2.0 20060117 (experimental)

this patch ICEs recent x86-64 gcc:

$ ./xgcc -B. -v
Reading specs from ./specs
Target: x86_64-pld-linux
Configured with: ../configure --prefix=/usr --with-local-prefix=/usr/local
--libdir=/usr/lib64 --libexecdir=/usr/lib64 --infodir=/usr/share/info
--mandir=/usr/share/man --x-libraries=/usr/lib64 --enable-shared
--enable-threads=posix --enable-languages=c,c++ --enable-c99 --enable-long-long
--enable-multilib --enable-nls --disable-werror --with-gnu-as --with-gnu-ld
--with-demangler-in-ld --with-system-zlib --with-slibdir=/lib64
--without-system-libunwind --without-x --with-long-double-128
--with-gxx-include-dir=/usr/include/c++/4.2.0 --disable-libstdcxx-pch
--enable-__cxa_atexit --enable-libstdcxx-allocator=new x86_64-pld-linux
Thread model: posix
gcc version 4.2.0 20060428 (experimental) (PLD-Linux)

$ cat T1.c
void test(double x) { if (x > 0.0); }

$ ./xgcc -B. T1.c -m32
T1.c: In function ‘test’:
T1.c:1: internal compiler error: in bsi_last, at tree-flow-inline.h:683

$ cat T2.cpp
struct S { ~S(); };
void bar();
void foo()
{
  S s;
  bar();
}

$ ./xgcc -B. T2.cpp -m32
T2.cpp: In function ‘void foo()’:
T2.cpp:7: internal compiler error: in bsi_last, at tree-flow-inline.h:683


-- 

pluto at agmk dot net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pluto at agmk dot net


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

[Bug target/17390] missing floating point compare optimization

[pluto at agmk dot net]

------- Comment #11 from pluto at agmk dot net  2006-07-25 11:33 -------
ping


-- 


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

[Bug target/17390] missing floating point compare optimization

[fxcoudert at gcc dot gnu dot org]

------- Comment #12 from fxcoudert at gcc dot gnu dot org  2007-05-10 23:15 -------
ping


-- 


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

[Bug target/17390] missing floating point compare optimization

[ubizjak at gmail dot com]

------- Comment #13 from ubizjak at gmail dot com  2007-05-18 15:13 -------
(In reply to comment #12)
> ping

This patch needs to be ported to dataflow infrastructure [1] and has to be
re-thought a bit.

[1]: http://gcc.gnu.org/ml/gcc-patches/2006-05/msg00040.html


-- 


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

[Bug target/17390] missing floating point compare optimization

[ubizjak at gmail dot com]

------- Comment #14 from ubizjak at gmail dot com  2007-08-23 08:33 -------
Patch at http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01513.html.

This is target-only patch that fakes fcomi instructions. It doesn't need to
rescan insn stream and creates the same output as in Comment #8. It also
doesn't ice for the testcase in Comment #10.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://gcc.gnu.org/ml/gcc-  |http://gcc.gnu.org/ml/gcc-
                   |patches/2005-               |patches/2007-
                   |10/msg01298.html            |08/msg01513.html


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

[Bug target/17390] missing floating point compare optimization

[uros at gcc dot gnu dot org]

------- Comment #15 from uros at gcc dot gnu dot org  2007-08-23 14:23 -------
Subject: Bug 17390

Author: uros
Date: Thu Aug 23 14:23:40 2007
New Revision: 127742

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127742
Log:
        PR target/17390
        * config/i386/i386.c (ix86_expand_fp_compare): Expand fp comparison to
        fake fcomi i387 instruction for !TARGET_CMOVE.
        (ix86_expand_branch): Expand natural sequence with one jump for
        all targets, not only TARGET_CMOVE.
        * config/i386/i386.md (*cmpfp_0_cc): New define_insn_and_split
        pattern to implement fake fcomi sequence.  Split instruction after
        reload to correct compare sequences.
        (*cmpfp_xf_cc): Ditto.
        (*cmpfp_<mode>_cc): Ditto.
        (*cmpfp_u_cc): Ditto.
        (*cmpfp_<mode>_cc): Ditto.

testsuite/ChangeLog:

        PR target/17390
        * gcc.target/i386/pr17390.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr17390.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.md
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/17390] missing floating point compare optimization

[ubizjak at gmail dot com]

------- Comment #16 from ubizjak at gmail dot com  2007-08-23 14:26 -------
Fixed.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.3.0


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

[Bug target/17390] missing floating point compare optimization

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-09-24 13:13 -------
Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |missed-optimization
   Last reconfirmed|0000-00-00 00:00:00         |2004-09-24 13:13:01
               date|                            |


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

[Bug target/17390] missing floating point compare optimization

[hubicka at ucw dot cz]

------- Additional Comments From hubicka at ucw dot cz  2004-09-10 09:48 -------
Subject: Re:  missing floating point compare optimization

> 
> ------- Additional Comments From uros at kss-loka dot si  2004-09-10 08:39 -------
> Some comments here:
> 
> http://gcc.gnu.org/ml/gcc/2004-09/msg00413.html
> http://gcc.gnu.org/ml/gcc/2004-09/msg00514.html
> 
> For TARGET_CMOVE, resulting code is totally different:
> 
> test:
>         fldz
>         fldl    4(%esp)
>         fcomip  %st(1), %st
>         je      .L11
>         fstp    %st(0)
>         flds    .LC1
>         fld1
>         fcmovbe %st(1), %st
>         fstp    %st(1)
> .L11:
>         ret
> 
> It looks that the problem is because  ix86_split_fp_branch() from *fp_jcc_?
> patterns is called after "reload_completed". However, the comment on top says:
> 
> ;; Define combination compare-and-branch fp compare instructions to use
> ;; during early optimization.  Splitting the operation apart early makes
> ;; for bad code when we want to reverse the operation.
> 
> It is not clear to me, what is meant by "reverse the operation"...

If you throw unwound sequence to the compiler early (having all the
funky bits with 387 control word, stores to memory and stuff..) you end
up with compiler no longer being able to modify the conditional in any
resonable way.
It is quite important to be able to reverse the conditional, swap
operands and determine constant result during the optimization.

The code here simply makes "fake" compare & branch FP instruction that
is later in the game decomposed into the true magic.  This idea however
unfortunately kills the idea of reusing the control word for multiple
comparisons.  Simply this scheme trades this quite uncommon
transformation for being able to do something with the conditionals.
Actually with tree-ssa we might try to revisit the idea - you might just
play around with the splitting early and doing some benchmark what
difference it still makes.

Things gets easier when fcomip instruction is available, I think we can
just try expose the decoposed sequence then as it is plain
compare&branch sequence in the case this is not done already.

Honza
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17390


-- 


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

Re: [Bug target/17390] missing floating point compare optimization

[Jan Hubicka]

> 
> ------- Additional Comments From uros at kss-loka dot si  2004-09-10 08:39 -------
> Some comments here:
> 
> http://gcc.gnu.org/ml/gcc/2004-09/msg00413.html
> http://gcc.gnu.org/ml/gcc/2004-09/msg00514.html
> 
> For TARGET_CMOVE, resulting code is totally different:
> 
> test:
>         fldz
>         fldl    4(%esp)
>         fcomip  %st(1), %st
>         je      .L11
>         fstp    %st(0)
>         flds    .LC1
>         fld1
>         fcmovbe %st(1), %st
>         fstp    %st(1)
> .L11:
>         ret
> 
> It looks that the problem is because  ix86_split_fp_branch() from *fp_jcc_?
> patterns is called after "reload_completed". However, the comment on top says:
> 
> ;; Define combination compare-and-branch fp compare instructions to use
> ;; during early optimization.  Splitting the operation apart early makes
> ;; for bad code when we want to reverse the operation.
> 
> It is not clear to me, what is meant by "reverse the operation"...

If you throw unwound sequence to the compiler early (having all the
funky bits with 387 control word, stores to memory and stuff..) you end
up with compiler no longer being able to modify the conditional in any
resonable way.
It is quite important to be able to reverse the conditional, swap
operands and determine constant result during the optimization.

The code here simply makes "fake" compare & branch FP instruction that
is later in the game decomposed into the true magic.  This idea however
unfortunately kills the idea of reusing the control word for multiple
comparisons.  Simply this scheme trades this quite uncommon
transformation for being able to do something with the conditionals.
Actually with tree-ssa we might try to revisit the idea - you might just
play around with the splitting early and doing some benchmark what
difference it still makes.

Things gets easier when fcomip instruction is available, I think we can
just try expose the decoposed sequence then as it is plain
compare&branch sequence in the case this is not done already.

Honza
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17390

[Bug target/17390] missing floating point compare optimization

[uros at kss-loka dot si]

------- Additional Comments From uros at kss-loka dot si  2004-09-10 08:39 -------
Some comments here:

http://gcc.gnu.org/ml/gcc/2004-09/msg00413.html
http://gcc.gnu.org/ml/gcc/2004-09/msg00514.html

For TARGET_CMOVE, resulting code is totally different:

test:
        fldz
        fldl    4(%esp)
        fcomip  %st(1), %st
        je      .L11
        fstp    %st(0)
        flds    .LC1
        fld1
        fcmovbe %st(1), %st
        fstp    %st(1)
.L11:
        ret

It looks that the problem is because  ix86_split_fp_branch() from *fp_jcc_?
patterns is called after "reload_completed". However, the comment on top says:

;; Define combination compare-and-branch fp compare instructions to use
;; during early optimization.  Splitting the operation apart early makes
;; for bad code when we want to reverse the operation.

It is not clear to me, what is meant by "reverse the operation"...

-- 


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