On Thu, 10 May 2012, Richard Guenther wrote:

> On Wed, 9 May 2012, Eric Botcazou wrote:
> 
> > > This removes the TYPE_IS_SIZETYPE macro and all its uses (by
> > > assuming it returns zero and applying trivial folding).  Sizes
> > > and bitsizes can still be treat specially by means of knowing
> > > what the values represent and by means of using helper functions
> > > that assume you are dealing with "sizes" (in particular size_binop
> > > and friends and bit_from_pos, byte_from_pos or pos_from_bit).
> > 
> > Fine with me, if you add the blurb I talked about in the other reply.
> > 
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages
> > > including Ada with the patch optimizing bute_from_pos and pos_from_bit
> > 
> > Results on our internal testsuite are clean on x86-64 and almost clean on 
> > x86, 
> > an exception being:
> > 
> > package t is
> >     type x (m : natural) is record
> >         s : string (1 .. m);
> >         r : natural;
> >         b : boolean;
> >     end record;
> >     for x'alignment use 4;
> > 
> >     pragma Pack (x);
> > end t;
> > 
> > Without the patches, compiling the package with -gnatR3 yields:
> > 
> > Representation information for unit t (spec)
> > --------------------------------------------
> > 
> > for x'Object_Size use 17179869248;
> > for x'Value_Size use  ((#1 + 8) * 8) ;
> > for x'Alignment use 4;
> > for x use record
> >    m at  0 range  0 .. 30;
> >    s at  4 range  0 ..  ((#1 * 8))  - 1;
> >    r at bit offset (((#1 + 4) * 8))  size in bits = 31
> >    b at bit offset ((((#1 + 7) * 8) + 7))  size in bits = 1
> > end record;
> > 
> > With the patches, this yields:
> > 
> > Representation information for unit t (spec)
> > --------------------------------------------
> > 
> > for x'Object_Size use 17179869248;
> > for x'Value_Size use  (((#1 + 7) + 1) * 8) ;
> > for x'Alignment use 4;
> > for x use record
> >    m at  0 range  0 .. 30;
> >    s at  4 range  0 ..  ((#1 * 8))  - 1;
> >    r at bit offset (((#1 + 4) * 8))  size in bits = 31
> >    b at bit offset ((((#1 + 7) * 8) + 7))  size in bits = 1
> > end record;
> > 
> > so we have lost a simple folding for x'Value_Size (TYPE_ADA_SIZE field).
> 
> That's interesting.  It is always safe to fold (x + 7) + 1 to
> (x + 8), independent on whether overflow is defined or not.  So this
> looks like a genuine missed folding (I think that the combiner
> in tree-ssa-forwprop.c catches this).  Or is the above not showing
> casts in the expression?  Folding would be not valid for
> (unsigned)(signed X + 7) + 1.

As far as I can see this happens when we fold

 (bitsizetype) (#1 + 7) * 8 + 7  PLUS_EXPR  1

which we fold to

 ((bitsizetype) (#1 + 7) + 1) * 8

The #1 + 7 expression is computed in sizetype (which is now unsigned
and thus has defined overflow - thus we cannot optimize the widening
to bitsizetype).

Equivalent C testcase:

unsigned long long foo (unsigned int x)
{
  return ((unsigned long long)(x + 7)) + 1;
}

As I previously suggested we can put in special knowledge into
size_binop, or maybe better, provide abstraction for conversion
of sizetype to bitsizetype that would associate the type
conversions.  The original plan was of course to at some point
have PLUSNV_EXPR so we can explicitely mark #1 + 7 as not
overflowing.  It might be that introducing those just for
size expressions right now (and then dropping them down
to regular PLUS_EXPRs during gimplification) might be
something to explore for 4.8.

Richard.

> > > 2012-05-08  Richard Guenther  <rguent...@suse.de>
> > >
> > >   ada/
> > >   * gcc-interface/cuintp.c (UI_From_gnu): Remove TYPE_IS_SIZETYPE use.
> > 
> > OK, modulo the formatting:
> 
> Adjusted and applied.
> 
> Thanks,
> Richard.
> 
> > > Index: trunk/gcc/ada/gcc-interface/cuintp.c
> > > ===================================================================
> > > *** trunk.orig/gcc/ada/gcc-interface/cuintp.c     2011-04-11 
> > > 17:01:30.000000000
> > > +0200 --- trunk/gcc/ada/gcc-interface/cuintp.c    2012-05-07
> > > 16:43:43.497218058 +0200 *************** UI_From_gnu (tree Input)
> > > *** 178,186 ****
> > >     if (host_integerp (Input, 0))
> > >       return UI_From_Int (TREE_INT_CST_LOW (Input));
> > >     else if (TREE_INT_CST_HIGH (Input) < 0
> > > !            && TYPE_UNSIGNED (gnu_type)
> > > !            && !(TREE_CODE (gnu_type) == INTEGER_TYPE
> > > !                 && TYPE_IS_SIZETYPE (gnu_type)))
> > >       return No_Uint;
> > >   #endif
> > >
> > > --- 178,184 ----
> > >     if (host_integerp (Input, 0))
> > >       return UI_From_Int (TREE_INT_CST_LOW (Input));
> > >     else if (TREE_INT_CST_HIGH (Input) < 0
> > > !            && TYPE_UNSIGNED (gnu_type))
> > >       return No_Uint;
> > >   #endif
> > 
> > && TYPE_UNSIGNED (gnu_type)) on the same line.
> 

-- 
Richard Guenther <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Reply via email to