diff mbox

Remove TYPE_IS_SIZETYPE

Message ID CAFiYyc0+mqhbDaHkXsx4_HTtKvcfoQHdeJWnm_VnSjBHYTm=Vw@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener May 10, 2012, noon UTC 
On Thu, May 10, 2012 at 12:26 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 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).
>
> I see, thanks for the investigation.
>
>> 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.
>
> OK, I'll think about it.  No objections by me to going ahead with the patches.

For example


fixes the specific testcase you provided.  I suppose if stor-layout.c would
be more carefully handle advancing offset/bitpos, avoding repeated translations
between them, those issues would not exist.  Of course the mere existence
of DECL_OFFSET_ALIGN complicates matters for no good reasons (well,
at least I did not find a good use of it until now ...).

Richard.

> --
> Eric Botcazou

Comments

Eric Botcazou May 10, 2012, 4:08 p.m. UTC | #1 
> For example
>
> Index: stor-layout.c
> ===================================================================
> --- stor-layout.c	(revision 187364)
> +++ stor-layout.c	(working copy)
> @@ -791,6 +791,10 @@ start_record_layout (tree t)
>  tree
>  bit_from_pos (tree offset, tree bitpos)
>  {
> +  if (TREE_CODE (offset) == PLUS_EXPR)
> +    offset = size_binop (PLUS_EXPR,
> +			 fold_convert (bitsizetype, TREE_OPERAND (offset, 0)),
> +			 fold_convert (bitsizetype, TREE_OPERAND (offset, 1)));
>    return size_binop (PLUS_EXPR, bitpos,
>  		     size_binop (MULT_EXPR,
>  				 fold_convert (bitsizetype, offset),
>
> fixes the specific testcase you provided.

I get a bootstrap failure on x86 (verify_flow_info failed) with it.  Let's drop 
it for now, we'll revisit this later.

> I suppose if stor-layout.c would 
> be more carefully handle advancing offset/bitpos, avoding repeated
> translations between them, those issues would not exist.  Of course the
> mere existence of DECL_OFFSET_ALIGN complicates matters for no good reasons
> (well, at least I did not find a good use of it until now ...).

Maybe it's also obsolete by now.
Olivier Hainque May 11, 2012, 2:01 p.m. UTC | #2 
On May 10, 2012, at 14:00 , Richard Guenther wrote:
> Of course the mere existence
> of DECL_OFFSET_ALIGN complicates matters for no good reasons (well,
> at least I did not find a good use of it until now ...).

I remember an old discussion about it ... hmm ...
 
   http://gcc.gnu.org/ml/gcc/2006-10/msg00019.html

 JIC that could be of use.

 Olivier
Richard Biener May 11, 2012, 2:06 p.m. UTC | #3 
On Fri, 11 May 2012, Olivier Hainque wrote:

> 
> On May 10, 2012, at 14:00 , Richard Guenther wrote:
> > Of course the mere existence
> > of DECL_OFFSET_ALIGN complicates matters for no good reasons (well,
> > at least I did not find a good use of it until now ...).
> 
> I remember an old discussion about it ... hmm ...
>  
>    http://gcc.gnu.org/ml/gcc/2006-10/msg00019.html
> 
>  JIC that could be of use.

Of course we just recently found out that DECL_OFFSET_ALIGN is
relative to the structure start - thus for Ada bit-aligned
sturctures it is "nonsense" (it says 128 but in fact the
offset is at 1 bit alignment).

Richard.
diff mbox

Patch

Index: stor-layout.c
===================================================================
--- stor-layout.c	(revision 187364)
+++ stor-layout.c	(working copy)
@@ -791,6 +791,10 @@  start_record_layout (tree t)
 tree
 bit_from_pos (tree offset, tree bitpos)
 {
+  if (TREE_CODE (offset) == PLUS_EXPR)
+    offset = size_binop (PLUS_EXPR,
+			 fold_convert (bitsizetype, TREE_OPERAND (offset, 0)),
+			 fold_convert (bitsizetype, TREE_OPERAND (offset, 1)));
   return size_binop (PLUS_EXPR, bitpos,
 		     size_binop (MULT_EXPR,
 				 fold_convert (bitsizetype, offset),