public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
@ 2011-12-14 22:46 ` bergner at gcc dot gnu.org
  2012-01-27 15:31 ` bergner at gcc dot gnu.org
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2011-12-14 22:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Peter Bergner <bergner at gcc dot gnu.org> 2011-12-14 22:42:57 UTC ---
Created attachment 26091
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26091
Another partial fix...

Answering Nathan's comment from his patch:

+    /* These are unsigned values, perhaps there will be a later
+       ordering compare that can be shared with this one.
+       Unfortunately we cannot detect the signedness of the operands
+       for non-subregs.  */

Actually, there is a way to tell whether some of our non-subreg operands are
unsigned or not.  For register operands of user defined variables:

  TYPE_UNSIGNED (TREE_TYPE (REG_EXPR (rtx)))

will be true if the operand is unsigned.  I've attached a patch that implements
that and it does fix the test case.  It also fixes a problem I encountered with
switch statements and -fno-jump-tables:

  enum CASE_VALUES {
    CASE0 = 0,
    CASE1,
    CASE2,
    CASE3,
    CASE4
  };

  int
  test_switch (enum CASE_VALUES index)
  {
    switch (index)
      {
        case CASE0: return case0 ();
        case CASE1: return case1 ();
        case CASE2: return case2 ();
        case CASE3: return case3 ();
        case CASE4: return case4 ();
        default:    return cased ();
      }
  }

Currently, GCC generates (-fno-jump-tables) the following, because it notices
that the enum contains no negative values, so it marks index as unsigned and we
end up with unsigned compares (modulo the equality compares) so we cannot CSE
them:

    test_switch:
        cmpwi 7,3,2
        beq 7,.L5
        cmplwi 7,3,2
        ble 7,.L10
        cmpwi 7,3,3
        beq 7,.L6
        ...

With the attached patch, we generate all unsigned compares and CSE is able to
remove the redundant compares:

    test_switch:
        cmplwi 7,3,2
        beq 7,.L5
        ble 7,.L10
        cmplwi 7,3,3
        beq 7,.L6
        ...

The patch still does not handle:

  if (*a == *b)
    ...
  else if (*a > *b)
    ...

or:

  switch (*index)
  ...

but we couldn't handle those before anyway.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug middle-end/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
  2011-12-14 22:46 ` [Bug middle-end/16458] PowerPC - redundant compare bergner at gcc dot gnu.org
@ 2012-01-27 15:31 ` bergner at gcc dot gnu.org
  2012-02-24 17:43 ` [Bug target/16458] " bergner at gcc dot gnu.org
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-01-27 15:31 UTC (permalink / raw)
  To: gcc-bugs

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |bergner at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from Peter Bergner <bergner at gcc dot gnu.org> 2012-01-27 15:12:15 UTC ---
Created attachment 26481
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26481
Updated patch to eliminate redudant compares

Here's a slightly updated patch that allows the compares to be cse'd, but also
doesn't disable the generation of record form instructions (ie, instructions
that also set cr0 implicitly).

I was mistaken in my Comment #5 above.  We can sometime handle:

  if (*a == *b)
    ...
  else if (*a > *b)
    ...

and:

  switch (*index)
  ...

depending on whether expand creates the type expression via REG_EXPR or not. 
In some cases it does and in others it doesn't.  That's not a limitation in
this patch, but in expand.  That can can be handled in separate patch.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug target/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
  2011-12-14 22:46 ` [Bug middle-end/16458] PowerPC - redundant compare bergner at gcc dot gnu.org
  2012-01-27 15:31 ` bergner at gcc dot gnu.org
@ 2012-02-24 17:43 ` bergner at gcc dot gnu.org
  2012-04-11 11:52 ` bergner at gcc dot gnu.org
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-02-24 17:43 UTC (permalink / raw)
  To: gcc-bugs

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

--- Comment #7 from Peter Bergner <bergner at gcc dot gnu.org> 2012-02-24 17:20:18 UTC ---
Richi mentioned this isn't suitable for 4.7 stage4, so re-targeting this for
4.8.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug target/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
                   ` (2 preceding siblings ...)
  2012-02-24 17:43 ` [Bug target/16458] " bergner at gcc dot gnu.org
@ 2012-04-11 11:52 ` bergner at gcc dot gnu.org
  2012-04-11 12:00 ` bergner at gcc dot gnu.org
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-04-11 11:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Peter Bergner <bergner at gcc dot gnu.org> 2012-04-11 11:51:54 UTC ---
Author: bergner
Date: Wed Apr 11 11:51:50 2012
New Revision: 186312

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186312
Log:
gcc/
    PR target/16458
    * rtlanal.c (unsigned_reg_p): New function.
    Update copyright notice dates.
    * rtl.h (unsigned_reg_p): Prototype it.
    Update copyright notice dates.
    * config/rs6000/rs6000.c (rs6000_generate_compare): Use it.
    Update comment.
    * expr.c (expand_expr_real_1): Set register attributes.
    * stmt.c (expand_case): Likewise.

gcc/testsuite/
    PR target/16458
    * gcc.target/powerpc/pr16458-1.c: New test.
    * gcc.target/powerpc/pr16458-2.c: Likewise.
    * gcc.target/powerpc/pr16458-3.c: Likewise.
    * gcc.target/powerpc/pr16458-4.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/powerpc/pr16458-1.c
    trunk/gcc/testsuite/gcc.target/powerpc/pr16458-2.c
    trunk/gcc/testsuite/gcc.target/powerpc/pr16458-3.c
    trunk/gcc/testsuite/gcc.target/powerpc/pr16458-4.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
    trunk/gcc/expr.c
    trunk/gcc/rtl.h
    trunk/gcc/rtlanal.c
    trunk/gcc/stmt.c
    trunk/gcc/testsuite/ChangeLog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug target/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
                   ` (3 preceding siblings ...)
  2012-04-11 11:52 ` bergner at gcc dot gnu.org
@ 2012-04-11 12:00 ` bergner at gcc dot gnu.org
  2012-06-02 20:29 ` bergner at gcc dot gnu.org
  2013-02-28 20:21 ` bergner at gcc dot gnu.org
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-04-11 12:00 UTC (permalink / raw)
  To: gcc-bugs

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

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
                URL|http://gcc.gnu.org/ml/gcc-p |http://gcc.gnu.org/ml/gcc-p
                   |atches/2012-01/msg01486.htm |atches/2012-04/msg00553.htm
                   |l                           |l
         Resolution|                            |FIXED

--- Comment #9 from Peter Bergner <bergner at gcc dot gnu.org> 2012-04-11 11:59:55 UTC ---
Fixed with the updated URL above.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug target/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
                   ` (4 preceding siblings ...)
  2012-04-11 12:00 ` bergner at gcc dot gnu.org
@ 2012-06-02 20:29 ` bergner at gcc dot gnu.org
  2013-02-28 20:21 ` bergner at gcc dot gnu.org
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2012-06-02 20:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Peter Bergner <bergner at gcc dot gnu.org> 2012-06-02 20:29:04 UTC ---
Author: bergner
Date: Sat Jun  2 20:28:52 2012
New Revision: 188141

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188141
Log:
    Backport from mainline
    2012-04-11  Peter Bergner  <bergner@vnet.ibm.com>
            Michael Matz  <matz@suse.de>

    PR target/16458
    * rtlanal.c (unsigned_reg_p): New function.
    Update copyright notice dates.
    * rtl.h (unsigned_reg_p): Prototype it.
    Update copyright notice dates.
    * config/rs6000/rs6000.c (rs6000_generate_compare): Use it.
    Update comment.
    * expr.c (expand_expr_real_1): Set register attributes.
    * stmt.c (expand_case): Likewise.

Added:
    branches/ibm/gcc-4_6-branch/gcc/testsuite/gcc.target/powerpc/pr16458-1.c
    branches/ibm/gcc-4_6-branch/gcc/testsuite/gcc.target/powerpc/pr16458-2.c
    branches/ibm/gcc-4_6-branch/gcc/testsuite/gcc.target/powerpc/pr16458-3.c
    branches/ibm/gcc-4_6-branch/gcc/testsuite/gcc.target/powerpc/pr16458-4.c
Modified:
    branches/ibm/gcc-4_6-branch/gcc/ChangeLog.ibm
    branches/ibm/gcc-4_6-branch/gcc/REVISION
    branches/ibm/gcc-4_6-branch/gcc/config/rs6000/rs6000.c
    branches/ibm/gcc-4_6-branch/gcc/expr.c
    branches/ibm/gcc-4_6-branch/gcc/rtl.h
    branches/ibm/gcc-4_6-branch/gcc/rtlanal.c
    branches/ibm/gcc-4_6-branch/gcc/stmt.c


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Bug target/16458] PowerPC - redundant compare
       [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
                   ` (5 preceding siblings ...)
  2012-06-02 20:29 ` bergner at gcc dot gnu.org
@ 2013-02-28 20:21 ` bergner at gcc dot gnu.org
  6 siblings, 0 replies; 7+ messages in thread
From: bergner at gcc dot gnu.org @ 2013-02-28 20:21 UTC (permalink / raw)
  To: gcc-bugs


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

--- Comment #11 from Peter Bergner <bergner at gcc dot gnu.org> 2013-02-28 20:21:05 UTC ---
Author: bergner
Date: Thu Feb 28 20:20:56 2013
New Revision: 196357

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196357
Log:
gcc/
    Backport from mainline
    2012-04-11  Peter Bergner  <bergner@vnet.ibm.com>
            Michael Matz  <matz@suse.de>

    PR target/16458
    * rtlanal.c (unsigned_reg_p): New function.
    Update copyright notice dates.
    * rtl.h (unsigned_reg_p): Prototype it.
    Update copyright notice dates.
    * config/rs6000/rs6000.c (rs6000_generate_compare): Use it.
    Update comment.
    * expr.c (expand_expr_real_1): Set register attributes.
    * stmt.c (expand_case): Likewise.

gcc/testsuite/
    Backport from mainline
    2012-04-11  Peter Bergner  <bergner@vnet.ibm.com>

    PR target/16458
    * gcc.target/powerpc/pr16458-1.c: New test.
    * gcc.target/powerpc/pr16458-2.c: Likewise.
    * gcc.target/powerpc/pr16458-3.c: Likewise.
    * gcc.target/powerpc/pr16458-4.c: Likewise.

Added:
    branches/ibm/gcc-4_7-branch/gcc/testsuite/gcc.target/powerpc/pr16458-1.c
    branches/ibm/gcc-4_7-branch/gcc/testsuite/gcc.target/powerpc/pr16458-2.c
    branches/ibm/gcc-4_7-branch/gcc/testsuite/gcc.target/powerpc/pr16458-3.c
    branches/ibm/gcc-4_7-branch/gcc/testsuite/gcc.target/powerpc/pr16458-4.c
Modified:
    branches/ibm/gcc-4_7-branch/gcc/ChangeLog.ibm
    branches/ibm/gcc-4_7-branch/gcc/config/rs6000/rs6000.c
    branches/ibm/gcc-4_7-branch/gcc/expr.c
    branches/ibm/gcc-4_7-branch/gcc/rtl.h
    branches/ibm/gcc-4_7-branch/gcc/rtlanal.c
    branches/ibm/gcc-4_7-branch/gcc/stmt.c
    branches/ibm/gcc-4_7-branch/gcc/testsuite/ChangeLog.ibm


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-02-28 20:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <bug-16458-4@http.gcc.gnu.org/bugzilla/>
2011-12-14 22:46 ` [Bug middle-end/16458] PowerPC - redundant compare bergner at gcc dot gnu.org
2012-01-27 15:31 ` bergner at gcc dot gnu.org
2012-02-24 17:43 ` [Bug target/16458] " bergner at gcc dot gnu.org
2012-04-11 11:52 ` bergner at gcc dot gnu.org
2012-04-11 12:00 ` bergner at gcc dot gnu.org
2012-06-02 20:29 ` bergner at gcc dot gnu.org
2013-02-28 20:21 ` bergner at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).