[Bug regression/16458] New: PowerPC - redundant compare

[Bug regression/16458] New: PowerPC - redundant compare

[gcc-bugzilla at gcc dot gnu dot org]

Description:
A non-optimal compare sequence is illustrated.  Two compares are being
done, one signed and the other unsigned.  One unsigned compare would
suffice.  Duplicate using gcc 3.5 and command line:

gcc -O3 -m64 -c test.c

Testcase:
unsigned int a, b;

int foo ()
{
  if (a == b) return 1;
  if (a > b)  return 2;
  if (a < b)  return 3;
  if (a != b) return 4;
  return 0;
}

Assembly:
      ld 9,.LC1@toc(2)
      li 3,1
      ld 11,.LC0@toc(2)
      lwz 10,0(9)
      lwz 0,0(11)
      cmpw 7,0,10   - signed compare of a and b
      cmplw 6,0,10  - unsigned compare of a and b
      beqlr- 7      - (a == b uses signed compare)
      li 3,2
      bgtlr- 6      - (a > b uses unsigned compare)
      li 3,4
      bgelr- 6      - (a != b uses unsigned compare)
      li 3,3
      blr



-- 
           Summary: PowerPC - redundant compare
           Product: gcc
           Version: 3.5.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P1
         Component: regression
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: steinmtz at us dot ibm dot com
                CC: gcc-bugs at gcc dot gnu dot org,steinmtz at us dot ibm
                    dot com
 GCC build triplet: powerpc64-linux
  GCC host triplet: powerpc64-linux
GCC target triplet: powerpc64-linux


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

[Bug rtl-optimization/16458] PowerPC - redundant compare

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-07-09 18:42 -------
Confirmed, any PPC has this trouble, 32 bit or not.
This looks more like a middle-end problem or a target because when converting unsigned == 
unsigned from tree to RTL, it changes it to be a signed comparison.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|regression                  |rtl-optimization
     Ever Confirmed|                            |1
  GCC build triplet|powerpc64-linux             |
   GCC host triplet|powerpc64-linux             |
 GCC target triplet|powerpc64-linux             |powerpc*-*
           Keywords|                            |missed-optimization
   Last reconfirmed|0000-00-00 00:00:00         |2004-07-09 18:42:00
               date|                            |


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

[Bug middle-end/16458] PowerPC - redundant compare

[nathan at gcc dot gnu dot org]

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


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

[Bug middle-end/16458] PowerPC - redundant compare

[nathan at gcc dot gnu dot org]

------- Additional Comments From nathan at gcc dot gnu dot org  2004-11-11 08:39 -------
2004-11-11  Nathan Sidwell  <nathan@codesourcery.com>

	PR target/16458
	* config/rs6000/rs6000.c (rs6000_generate_compare): Generate an
	unsigned equality compare when we know the operands are unsigned.

this is a partial fix, the remainder is dependent on 18395, IMO

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |18395
         AssignedTo|nathan at gcc dot gnu dot   |unassigned at gcc dot gnu
                   |org                         |dot org
             Status|ASSIGNED                    |NEW


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

[Bug middle-end/16458] PowerPC - redundant compare

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-11-11 08:41 -------
Subject: Bug 16458

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	nathan@gcc.gnu.org	2004-11-11 08:40:43

Modified files:
	gcc            : ChangeLog 
	gcc/config/rs6000: rs6000.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg: ppc-compare-1.c 

Log message:
	.:
	PR target/16458
	* config/rs6000/rs6000.c (rs6000_generate_compare): Generate an
	unsigned equality compare when we know the operands are unsigned.
	testsuite:
	PR target/16458
	* gcc.dg/ppc-compare-1.c: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.6271&r2=2.6272
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/rs6000/rs6000.c.diff?cvsroot=gcc&r1=1.739&r2=1.740
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4574&r2=1.4575
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/ppc-compare-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


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

[Bug middle-end/16458] PowerPC - redundant compare

[bergner at gcc dot gnu.org]

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.

[Bug middle-end/16458] PowerPC - redundant compare

[bergner at gcc dot gnu.org]

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.

[Bug middle-end/16458] PowerPC - redundant compare

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2005-11-02 17:16 -------
All P1 enhancements not targeted towards 4.1, moving to P5.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P5


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