[Bug middle-end/52134] New: Does not fold (x * 4) & -4

[Bug middle-end/52134] New: Does not fold (x * 4) & -4

[rguenth at gcc dot gnu.org]

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

             Bug #: 52134
           Summary: Does not fold (x * 4) & -4
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: enhancement
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rguenth@gcc.gnu.org
                CC: ebotcazou@gcc.gnu.org


We appearantly do not fold

((sizetype) MAX_EXPR <R1b, 0> + 2) * 4

BIT_AND_EXPR

-4

as it happens for TYPE_SIZE_UNIT of a struct for gnat.dg/opt9.adb.

This causes an issue with trying to compute the extent/padding of
a trailing bitfield in such a struct when you do

size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT (field)),
             DECL_FIELD_OFFSET (repr));

where repr is the first field of a bitfield group and field is the last
field of a bitfield group.

The above expression computes non-constant

((ssizetype) (((sizetype) MAX_EXPR <R1b, 0> + 2) * 4) & -4) - (ssizetype)
((sizetype) MAX_EXPR <R1b, 0> * 4)

Not sure if such an expression cannot be reliably required to be constant
in ada though.

Certainly this looks like a missed folding (not sure how often this
odd BIT_AND_EXPR for sizes happen in Ada).

[Bug middle-end/52134] Does not fold (x * 4) & -4

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-06
     Ever Confirmed|0                           |1

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-02-06 14:08:13 UTC ---
> The above expression computes non-constant
> 
> ((ssizetype) (((sizetype) MAX_EXPR <R1b, 0> + 2) * 4) & -4) - (ssizetype)
> ((sizetype) MAX_EXPR <R1b, 0> * 4)
> 
> Not sure if such an expression cannot be reliably required to be constant
> in ada though.

Reassociation/simplification of size expressions can generally be done at will.

> Certainly this looks like a missed folding (not sure how often this
> odd BIT_AND_EXPR for sizes happen in Ada).

Quite a lot, but it's created by the folder itself (round_up).

[Bug middle-end/52134] Does not fold (x * 4) & -4

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |TREE

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-02-06 19:45:41 UTC ---
Short testcase:
unsigned f(unsigned t)
{
  return (t*4)&-4;
}
int f1(int t)
{
  return (t*4)&-4;
}

Both should be optimized to just t*4.  In fact we do it on the RTL level.
*4 is changed to <<a 2 on the RTL level and simplified with the & when doing
combine.

[Bug middle-end/52134] Does not fold (x * 4) & -4

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

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

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-13 11:42:41 UTC ---
I have a patch.

[Bug middle-end/52134] Does not fold (x * 4) & -4

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-13 13:47:43 UTC ---
Author: rguenth
Date: Tue Mar 13 13:47:35 2012
New Revision: 185334

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185334
Log:
2012-03-13  Richard Guenther  <rguenther@suse.de>

    PR middle-end/52134
    * fold-const.c (fold_binary_loc): Fold (X * Y) & -(1 << CST) to X * Y
    if Y is a constant multiple of 1 << CST.

    * gcc.dg/pr52134.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/pr52134.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/52134] Does not fold (x * 4) & -4

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

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

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-13 13:54:19 UTC ---
Fixed.

[Bug middle-end/52134] Does not fold (x * 4) & -4

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-03-13 22:08:12 UTC ---
CCP could also remove the &:
Visiting statement:
D.1713_2 = t_1(D) * 4;
which is likely CONSTANT
Lattice value changed to CONSTANT Lattice value changed to CONSTANT
0x00000000000000000 (0x000000000fffffffc).  Adding SSA edges to worklist.

Visiting statement:
D.1712_3 = D.1713_2 & 4294967292;
which is likely CONSTANT
Lattice value changed to CONSTANT Lattice value changed to CONSTANT
0x00000000000000000 (0x000000000fffffffc).  Adding SSA edges to worklist.

..
Visiting statement:
D.1710_2 = t_1(D) * 4;
which is likely CONSTANT
Lattice value changed to CONSTANT Lattice value changed to CONSTANT
0x00000000000000000 (0xfffffffffffffffffffffffffffffffc).  Adding SSA edges to
worklist.

Visiting statement:
D.1709_3 = D.1710_2 & -4;
which is likely CONSTANT
Lattice value changed to CONSTANT Lattice value changed to CONSTANT
0x00000000000000000 (0xfffffffffffffffffffffffffffffffc).  Adding SSA edges to
worklist.


See how the lattice's already have its last 3 bits unset.  In fact I think we
should only do this in the ccp/vrp passes to remove the & rather than fold.

[Bug middle-end/52134] Does not fold (x * 4) & -4

[ebotcazou at gcc dot gnu.org]

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

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-03-13 22:17:35 UTC ---
> See how the lattice's already have its last 3 bits unset.  In fact I think we
> should only do this in the ccp/vrp passes to remove the & rather than fold.

For size calculations (TYPE_IS_SIZETYPE) the earlier you fold, the better, so
that you don't have to drag huge expressions for variable-sized types in the
FE.

[Bug middle-end/52134] Does not fold (x * 4) & -4

[rguenther at suse dot de]

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

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot de> 2012-03-14 09:39:31 UTC ---
On Tue, 13 Mar 2012, pinskia at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52134
> 
> --- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-03-13 22:08:12 UTC ---
> CCP could also remove the &:
> Visiting statement:
> D.1713_2 = t_1(D) * 4;
> which is likely CONSTANT
> Lattice value changed to CONSTANT Lattice value changed to CONSTANT
> 0x00000000000000000 (0x000000000fffffffc).  Adding SSA edges to worklist.
> 
> Visiting statement:
> D.1712_3 = D.1713_2 & 4294967292;
> which is likely CONSTANT
> Lattice value changed to CONSTANT Lattice value changed to CONSTANT
> 0x00000000000000000 (0x000000000fffffffc).  Adding SSA edges to worklist.
> 
> ..
> Visiting statement:
> D.1710_2 = t_1(D) * 4;
> which is likely CONSTANT
> Lattice value changed to CONSTANT Lattice value changed to CONSTANT
> 0x00000000000000000 (0xfffffffffffffffffffffffffffffffc).  Adding SSA edges to
> worklist.
> 
> Visiting statement:
> D.1709_3 = D.1710_2 & -4;
> which is likely CONSTANT
> Lattice value changed to CONSTANT Lattice value changed to CONSTANT
> 0x00000000000000000 (0xfffffffffffffffffffffffffffffffc).  Adding SSA edges to
> worklist.
> 
> 
> See how the lattice's already have its last 3 bits unset.  In fact I think we
> should only do this in the ccp/vrp passes to remove the & rather than fold.

Yeah, CCP and VRP can do this as well.  In fact folding bit-operations
with the bit-CCP lattice is an obvious improvement, best done in
ccp_fold_stmt which is called at lattice substitution time.

Richard.