public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR53501
@ 2012-05-30 10:42 Richard Guenther
  2012-05-31 12:31 ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Guenther @ 2012-05-30 10:42 UTC (permalink / raw)
  To: gcc-patches


This fixes PR53501, fold_plusminus_mult_expr does not expect that
operands have a sign-conversion stripped.  So don't call it with
such arguments.

Bootstrap and regtest pending on x86_64-unknown-linux-gnu.

Richard.

2012-05-30  Richard Guenther  <rguenther@suse.de>

	PR middle-end/53501
	* fold-const.c (fold_binary_loc): Make sure to call
	fold_plusminus_mult_expr with the original sign of operands.

	* gcc.dg/torture/pr53501.c: New testcase.

Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c	(revision 188004)
--- gcc/fold-const.c	(working copy)
*************** fold_binary_loc (location_t loc,
*** 10045,10056 ****
        /* Handle (A1 * C1) + (A2 * C2) with A1, A2 or C1, C2 being the
  	 same or one.  Make sure type is not saturating.
  	 fold_plusminus_mult_expr will re-associate.  */
!       if ((TREE_CODE (arg0) == MULT_EXPR
! 	   || TREE_CODE (arg1) == MULT_EXPR)
  	  && !TYPE_SATURATING (type)
  	  && (!FLOAT_TYPE_P (type) || flag_associative_math))
          {
! 	  tree tem = fold_plusminus_mult_expr (loc, code, type, arg0, arg1);
  	  if (tem)
  	    return tem;
  	}
--- 10045,10056 ----
        /* Handle (A1 * C1) + (A2 * C2) with A1, A2 or C1, C2 being the
  	 same or one.  Make sure type is not saturating.
  	 fold_plusminus_mult_expr will re-associate.  */
!       if ((TREE_CODE (op0) == MULT_EXPR
! 	   || TREE_CODE (op1) == MULT_EXPR)
  	  && !TYPE_SATURATING (type)
  	  && (!FLOAT_TYPE_P (type) || flag_associative_math))
          {
! 	  tree tem = fold_plusminus_mult_expr (loc, code, type, op0, op1);
  	  if (tem)
  	    return tem;
  	}
*************** fold_binary_loc (location_t loc,
*** 10668,10679 ****
        /* Handle (A1 * C1) - (A2 * C2) with A1, A2 or C1, C2 being the
  	 same or one.  Make sure type is not saturating.
  	 fold_plusminus_mult_expr will re-associate.  */
!       if ((TREE_CODE (arg0) == MULT_EXPR
! 	   || TREE_CODE (arg1) == MULT_EXPR)
  	  && !TYPE_SATURATING (type)
  	  && (!FLOAT_TYPE_P (type) || flag_associative_math))
          {
! 	  tree tem = fold_plusminus_mult_expr (loc, code, type, arg0, arg1);
  	  if (tem)
  	    return tem;
  	}
--- 10668,10679 ----
        /* Handle (A1 * C1) - (A2 * C2) with A1, A2 or C1, C2 being the
  	 same or one.  Make sure type is not saturating.
  	 fold_plusminus_mult_expr will re-associate.  */
!       if ((TREE_CODE (op0) == MULT_EXPR
! 	   || TREE_CODE (op1) == MULT_EXPR)
  	  && !TYPE_SATURATING (type)
  	  && (!FLOAT_TYPE_P (type) || flag_associative_math))
          {
! 	  tree tem = fold_plusminus_mult_expr (loc, code, type, op0, op1);
  	  if (tem)
  	    return tem;
  	}
Index: gcc/testsuite/gcc.dg/torture/pr53501.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr53501.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr53501.c	(revision 0)
***************
*** 0 ****
--- 1,22 ----
+ /* { dg-do run } */
+ 
+ extern void abort (void);
+ 
+ int e[100], n, here;
+ 
+ void __attribute__((noinline))
+ foo(void)
+ {
+   int i, k = 0;
+   for (i = 0; i < n; ++i) { e[k] = 10; ++k; e[k] = 10; ++k; }
+   for (i = 0; i < k; ++i) here = 1;
+   if (here != 1)
+     abort ();
+ }
+ 
+ int main(void)
+ {
+   n = 10;
+   foo();
+   return 0;
+ }

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

* Re: [PATCH] Fix PR53501
  2012-05-30 10:42 [PATCH] Fix PR53501 Richard Guenther
@ 2012-05-31 12:31 ` Eric Botcazou
  2012-05-31 12:38   ` Richard Guenther
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-05-31 12:31 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

> This fixes PR53501, fold_plusminus_mult_expr does not expect that
> operands have a sign-conversion stripped.  So don't call it with
> such arguments.
>
> Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2012-05-30  Richard Guenther  <rguenther@suse.de>
>
> 	PR middle-end/53501
> 	* fold-const.c (fold_binary_loc): Make sure to call
> 	fold_plusminus_mult_expr with the original sign of operands.

This pessimizes size computations in Ada for 64-bit targets on the 4.7 branch.

For the attached testcase, we go from:

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;

to

Representation information for unit t (spec)
--------------------------------------------

for x'Object_Size use 17179869248;
for x'Value_Size use  (((#1 + 7) * 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 -gnatR3 and I'm not sure to understand what the sign has to do here.

-- 
Eric Botcazou

[-- Attachment #2: t.ads --]
[-- Type: text/x-adasrc, Size: 193 bytes --]

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;

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

* Re: [PATCH] Fix PR53501
  2012-05-31 12:31 ` Eric Botcazou
@ 2012-05-31 12:38   ` Richard Guenther
  2012-06-01  8:49     ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Guenther @ 2012-05-31 12:38 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Thu, 31 May 2012, Eric Botcazou wrote:

> > This fixes PR53501, fold_plusminus_mult_expr does not expect that
> > operands have a sign-conversion stripped.  So don't call it with
> > such arguments.
> >
> > Bootstrap and regtest pending on x86_64-unknown-linux-gnu.
> >
> > Richard.
> >
> > 2012-05-30  Richard Guenther  <rguenther@suse.de>
> >
> > 	PR middle-end/53501
> > 	* fold-const.c (fold_binary_loc): Make sure to call
> > 	fold_plusminus_mult_expr with the original sign of operands.
> 
> This pessimizes size computations in Ada for 64-bit targets on the 4.7 branch.
> 
> For the attached testcase, we go from:
> 
> 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;
> 
> to
> 
> Representation information for unit t (spec)
> --------------------------------------------
> 
> for x'Object_Size use 17179869248;
> for x'Value_Size use  (((#1 + 7) * 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 -gnatR3 and I'm not sure to understand what the sign has to do here.

The issue is that fold_plusminus_mult re-writes the multiplication
from unsigned to signed for the failing testcase, introducing
undefined overflow.  Yes, it's possible to make fold_plusminus_mult
deal with the situation properly (you can always express the
computations in unsigned arithmetic if association simplifies it).
I just thought that's not appropriate for the branch, so the
simple wrong-code fix was better (the rest is on my TODO for trunk - if
you do not beat me to it).

Richard.

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

* Re: [PATCH] Fix PR53501
  2012-05-31 12:38   ` Richard Guenther
@ 2012-06-01  8:49     ` Eric Botcazou
  2012-06-01  8:52       ` Richard Guenther
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-06-01  8:49 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

> The issue is that fold_plusminus_mult re-writes the multiplication
> from unsigned to signed for the failing testcase, introducing
> undefined overflow.

fold_plusminus_mult or fold_binary?  My understanding is that the problem is 
fold_binary incorrectly stripping the outer signedness conversion.  If so, 
then the fix could test for this case precisely instead of disabling the 
transformation in all cases, including when there is no signedness conversion.

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-06-01  8:49     ` Eric Botcazou
@ 2012-06-01  8:52       ` Richard Guenther
  2012-06-01  9:40         ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Guenther @ 2012-06-01  8:52 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 1 Jun 2012, Eric Botcazou wrote:

> > The issue is that fold_plusminus_mult re-writes the multiplication
> > from unsigned to signed for the failing testcase, introducing
> > undefined overflow.
> 
> fold_plusminus_mult or fold_binary?  My understanding is that the problem is 
> fold_binary incorrectly stripping the outer signedness conversion.  If so, 
> then the fix could test for this case precisely instead of disabling the 
> transformation in all cases, including when there is no signedness conversion.

fold_binary is not "incorrectly" stripping the sign conversion, it is
fold_plusminus_mult not properly dealing with that situation.  In practice
non-sign-"nop"-conversions stipped by STRIP_NOPS are already stripped
by properly folding a conversion at generation time, so the STRIP_NOPS
calls should be no-ops (well, apart from stripping NON_LVALUE trees I 
suppose).

So, what case do you see disabled where there is no sign conversion
involved?

Richard.

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

* Re: [PATCH] Fix PR53501
  2012-06-01  8:52       ` Richard Guenther
@ 2012-06-01  9:40         ` Eric Botcazou
  2012-06-01  9:44           ` Richard Guenther
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-06-01  9:40 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

> So, what case do you see disabled where there is no sign conversion
> involved?

For the Ada testcase I attached, in fold_binary_loc we have:

(gdb) p debug_tree(op0)
 <nop_expr 0x7ffff700c5a0
    type <integer_type 0x7ffff6fdd0a8 bitsizetype public unsigned sizetype DI
        size <integer_cst 0x7ffff6fccec0 constant visited 64>
        unit size <integer_cst 0x7ffff6fccee0 constant visited 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd0a8 precision 
64 min <integer_cst 0x7ffff6fccf60 0> max <integer_cst 0x7ffff6fccf80 -1>>
    readonly
    arg 0 <mult_expr 0x7ffff700e030
        type <integer_type 0x7ffff6fdd000 sizetype public unsigned sizetype DI 
size <integer_cst 0x7ffff6fccec0 64> unit size <integer_cst 0x7ffff6fccee0 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd000 
precision 64 min <integer_cst 0x7ffff6fccf00 0> max <integer_cst 
0x7ffff6fccf20 -1>>
        readonly

(gdb) p debug_tree(arg0)
 <mult_expr 0x7ffff700e030
    type <integer_type 0x7ffff6fdd000 sizetype public unsigned sizetype DI
        size <integer_cst 0x7ffff6fccec0 constant visited 64>
        unit size <integer_cst 0x7ffff6fccee0 constant visited 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd000 precision 
64 min <integer_cst 0x7ffff6fccf00 0> max <integer_cst 0x7ffff6fccf20 -1>>
    readonly

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-06-01  9:40         ` Eric Botcazou
@ 2012-06-01  9:44           ` Richard Guenther
  2012-06-01  9:54             ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Guenther @ 2012-06-01  9:44 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 1 Jun 2012, Eric Botcazou wrote:

> > So, what case do you see disabled where there is no sign conversion
> > involved?
> 
> For the Ada testcase I attached, in fold_binary_loc we have:
> 
> (gdb) p debug_tree(op0)
>  <nop_expr 0x7ffff700c5a0
>     type <integer_type 0x7ffff6fdd0a8 bitsizetype public unsigned sizetype DI
>         size <integer_cst 0x7ffff6fccec0 constant visited 64>
>         unit size <integer_cst 0x7ffff6fccee0 constant visited 8>
>         align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd0a8 precision 
> 64 min <integer_cst 0x7ffff6fccf60 0> max <integer_cst 0x7ffff6fccf80 -1>>
>     readonly
>     arg 0 <mult_expr 0x7ffff700e030
>         type <integer_type 0x7ffff6fdd000 sizetype public unsigned sizetype DI 
> size <integer_cst 0x7ffff6fccec0 64> unit size <integer_cst 0x7ffff6fccee0 8>
>             align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd000 
> precision 64 min <integer_cst 0x7ffff6fccf00 0> max <integer_cst 
> 0x7ffff6fccf20 -1>>
>         readonly
> 
> (gdb) p debug_tree(arg0)
>  <mult_expr 0x7ffff700e030
>     type <integer_type 0x7ffff6fdd000 sizetype public unsigned sizetype DI
>         size <integer_cst 0x7ffff6fccec0 constant visited 64>
>         unit size <integer_cst 0x7ffff6fccee0 constant visited 8>
>         align 64 symtab 0 alias set -1 canonical type 0x7ffff6fdd000 precision 
> 64 min <integer_cst 0x7ffff6fccf00 0> max <integer_cst 0x7ffff6fccf20 -1>>
>     readonly

Ah, I see.  So the proper fix would be to use STRIP_NOP()ed op0/op1,
something not readily available though.

Richard.

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

* Re: [PATCH] Fix PR53501
  2012-06-01  9:44           ` Richard Guenther
@ 2012-06-01  9:54             ` Eric Botcazou
  2012-06-01 10:09               ` Richard Guenther
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-06-01  9:54 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

> Ah, I see.  So the proper fix would be to use STRIP_NOP()ed op0/op1,
> something not readily available though.

Why not just add

  TYPE_UNSIGNED (TREE_TYPE (op0)) == TYPE_UNSIGNED (TREE_TYPE (arg0))
  && TYPE_UNSIGNED (TREE_TYPE (op1)) == TYPE_UNSIGNED (TREE_TYPE (arg1))

with a small comment?

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-06-01  9:54             ` Eric Botcazou
@ 2012-06-01 10:09               ` Richard Guenther
  2012-06-01 10:31                 ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: Richard Guenther @ 2012-06-01 10:09 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 1 Jun 2012, Eric Botcazou wrote:

> > Ah, I see.  So the proper fix would be to use STRIP_NOP()ed op0/op1,
> > something not readily available though.
> 
> Why not just add
> 
>   TYPE_UNSIGNED (TREE_TYPE (op0)) == TYPE_UNSIGNED (TREE_TYPE (arg0))
>   && TYPE_UNSIGNED (TREE_TYPE (op1)) == TYPE_UNSIGNED (TREE_TYPE (arg1))
> 
> with a small comment?

Well, it would rather be

  TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg0))
  && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))

but only in the !FLOAT_TYPE_P path.  We could even compare
TYPE_OVERFLOW_UNDEFINED I think.  Or even just make sure
that when TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)) also
TYPE_OVERFLOW_UNDEFINED (type), thus

  !TYPE_OVERFLOW_UNDEFINED (type)
  || ((TREE_CODE (arg0) != MULT_EXPR
       || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
      && (TREE_CODE (arg1) != MULT_EXPR
          || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1))))

That is, the newly created multiplication in type TYPE should
either not have undefined overflow or the inner multiplications
all should already have.  Best done with a comment in
fold_plusminus_mult_expr.

Richard.

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

* Re: [PATCH] Fix PR53501
  2012-06-01 10:09               ` Richard Guenther
@ 2012-06-01 10:31                 ` Eric Botcazou
  2012-06-01 10:34                   ` Richard Guenther
  2012-10-31 10:30                   ` H.J. Lu
  0 siblings, 2 replies; 21+ messages in thread
From: Eric Botcazou @ 2012-06-01 10:31 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

> Well, it would rather be
>
>   TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg0))
>   && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))
>
> but only in the !FLOAT_TYPE_P path.

That works in all cases I think, see existing cases in the folder.

> We could even compare 
> TYPE_OVERFLOW_UNDEFINED I think.  Or even just make sure
> that when TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)) also
> TYPE_OVERFLOW_UNDEFINED (type), thus
>
>   !TYPE_OVERFLOW_UNDEFINED (type)
>
>   || ((TREE_CODE (arg0) != MULT_EXPR
>   ||
>        || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
>
>       && (TREE_CODE (arg1) != MULT_EXPR
>
>           || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1))))
>
> That is, the newly created multiplication in type TYPE should
> either not have undefined overflow or the inner multiplications
> all should already have.  Best done with a comment in
> fold_plusminus_mult_expr.

I'm a little lost here. :-)  I don't really care about the mainline at this 
point, but the fix on the branches should be the minimal working one.

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-06-01 10:31                 ` Eric Botcazou
@ 2012-06-01 10:34                   ` Richard Guenther
  2012-10-31 10:30                   ` H.J. Lu
  1 sibling, 0 replies; 21+ messages in thread
From: Richard Guenther @ 2012-06-01 10:34 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 1 Jun 2012, Eric Botcazou wrote:

> > Well, it would rather be
> >
> >   TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg0))
> >   && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))
> >
> > but only in the !FLOAT_TYPE_P path.
> 
> That works in all cases I think, see existing cases in the folder.

(*)

> > We could even compare 
> > TYPE_OVERFLOW_UNDEFINED I think.  Or even just make sure
> > that when TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)) also
> > TYPE_OVERFLOW_UNDEFINED (type), thus
> >
> >   !TYPE_OVERFLOW_UNDEFINED (type)
> >
> >   || ((TREE_CODE (arg0) != MULT_EXPR
> >   ||
> >        || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
> >
> >       && (TREE_CODE (arg1) != MULT_EXPR
> >
> >           || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1))))
> >
> > That is, the newly created multiplication in type TYPE should
> > either not have undefined overflow or the inner multiplications
> > all should already have.  Best done with a comment in
> > fold_plusminus_mult_expr.
> 
> I'm a little lost here. :-)  I don't really care about the mainline at this 
> point, but the fix on the branches should be the minimal working one.

Ok ... the (*) fix is ok then on both branch and trunk.

We can leave improving trunk for a followup.

Richard.

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

* Re: [PATCH] Fix PR53501
  2012-06-01 10:31                 ` Eric Botcazou
  2012-06-01 10:34                   ` Richard Guenther
@ 2012-10-31 10:30                   ` H.J. Lu
  2012-10-31 12:49                     ` Eric Botcazou
  1 sibling, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2012-10-31 10:30 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: Richard Guenther, gcc-patches

On Fri, Jun 1, 2012 at 3:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Well, it would rather be
>>
>>   TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg0))
>>   && TYPE_UNSIGNED (type) == TYPE_UNSIGNED (TREE_TYPE (arg1))
>>
>> but only in the !FLOAT_TYPE_P path.
>
> That works in all cases I think, see existing cases in the folder.
>
>> We could even compare
>> TYPE_OVERFLOW_UNDEFINED I think.  Or even just make sure
>> that when TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)) also
>> TYPE_OVERFLOW_UNDEFINED (type), thus
>>
>>   !TYPE_OVERFLOW_UNDEFINED (type)
>>
>>   || ((TREE_CODE (arg0) != MULT_EXPR
>>   ||
>>        || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg0)))
>>
>>       && (TREE_CODE (arg1) != MULT_EXPR
>>
>>           || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1))))
>>
>> That is, the newly created multiplication in type TYPE should
>> either not have undefined overflow or the inner multiplications
>> all should already have.  Best done with a comment in
>> fold_plusminus_mult_expr.
>
> I'm a little lost here. :-)  I don't really care about the mainline at this
> point, but the fix on the branches should be the minimal working one.
>

Your change caused:

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

-- 
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-10-31 10:30                   ` H.J. Lu
@ 2012-10-31 12:49                     ` Eric Botcazou
  2012-10-31 13:06                       ` H.J. Lu
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-10-31 12:49 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Richard Guenther

> Your change caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55142

Please check whether it worked before Richard's fix (r188009).

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-10-31 12:49                     ` Eric Botcazou
@ 2012-10-31 13:06                       ` H.J. Lu
  2012-10-31 14:41                         ` Eric Botcazou
  0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2012-10-31 13:06 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 5:42 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Your change caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55142
>
> Please check whether it worked before Richard's fix (r188009).
>

It failed with revision 188008.

-- 
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-10-31 13:06                       ` H.J. Lu
@ 2012-10-31 14:41                         ` Eric Botcazou
  2012-10-31 23:03                           ` H.J. Lu
  0 siblings, 1 reply; 21+ messages in thread
From: Eric Botcazou @ 2012-10-31 14:41 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Richard Guenther

> It failed with revision 188008.

OK, thanks.  So the testcase never compiled on the trunk (except for about 24 
hours between 188009 & 188118) or did it compile before 188008 at some point?

-- 
Eric Botcazou

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

* Re: [PATCH] Fix PR53501
  2012-10-31 14:41                         ` Eric Botcazou
@ 2012-10-31 23:03                           ` H.J. Lu
  2012-11-01  1:57                             ` H.J. Lu
  2012-11-01  8:43                             ` Eric Botcazou
  0 siblings, 2 replies; 21+ messages in thread
From: H.J. Lu @ 2012-10-31 23:03 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 7:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> > It failed with revision 188008.
>
> OK, thanks.  So the testcase never compiled on the trunk (except for about
> 24
> hours between 188009 & 188118) or did it compile before 188008 at some
> point?
>
> --
> Eric Botcazou

It was OK until revision 187042:

    2012-05-02  Richard Guenther  <rguenther@suse.de>

        * tree.c (valid_constant_size_p): New function.
        * tree.h (valid_constant_size_p): Declare.
        * cfgexpand.c (expand_one_var): Adjust check for too large
        variables by using valid_constant_size_p.
        * varasm.c (assemble_variable): Likewise.

        c/
        * c-decl.c (grokdeclarator): Properly check for sizes that
        cover more than half of the address-space.

        cp/
        * decl.c (grokdeclarator): Properly check for sizes that
        cover more than half of the address-space.

    2012-05-02  Richard Guenther  <rguenther@suse.de>

        * fold-const.c (div_if_zero_remainder): sizetypes no longer
        sign-extend.
        (int_const_binop_1): New worker for int_const_binop with
        overflowable parameter.  Pass it through
        to force_fit_type_double.
        (int_const_binop): Wrap around int_const_binop_1 with overflowable
        equal to one.
        (size_binop_loc): Call int_const_binop_1 with overflowable equal
        to minus one, forcing overflow detection for even unsigned types.
        (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
        (fold_binary_loc): Call try_move_mult_to_index with signed offset.
        * stor-layout.c (initialize_sizetypes): sizetypes no longer
        sign-extend.
        (layout_type): For zero-sized arrays ignore overflow on the
        size calculations.
        * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
        (bit_value_binop_1): Likewise.
        * tree.c (double_int_to_tree): Likewise.
        (double_int_fits_to_tree_p): Likewise.
        (force_fit_type_double): Likewise.
        (host_integerp): Likewise.
        (int_fits_type_p): Likewise.
        * varasm.c (output_constructor_regular_field): Sign-extend the
        field-offset to cater for negative offsets produced by the Ada frontend.
        * omp-low.c (extract_omp_for_data): Convert the loop step to
        signed for pointer adjustments.

which gave:

/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O3 -Wall -mx32
-maddress-mode=long -fPIC -S x.i
x.i: In function ‘dl_start’:
x.i:30:1: error: unrecognizable insn:
 }
 ^
(insn 54 53 55 7 (set (reg:SI 108)
        (const:SI (plus:SI (symbol_ref:SI ("_dl_rtld_map") [flags
0x42] <var_decl 0x7f02997bb140 _dl_rtld_map>)
                (const_int -1073742800 [0xffffffffbffffc30])))) x.i:22 -1
     (nil))
x.i:30:1: internal compiler error: in extract_insn, at recog.c:2130
 }
 ^
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.



--
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-10-31 23:03                           ` H.J. Lu
@ 2012-11-01  1:57                             ` H.J. Lu
  2012-11-01  2:14                               ` H.J. Lu
  2012-11-01  2:21                               ` Andrew Pinski
  2012-11-01  8:43                             ` Eric Botcazou
  1 sibling, 2 replies; 21+ messages in thread
From: H.J. Lu @ 2012-11-01  1:57 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 3:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 7:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>
>> > It failed with revision 188008.
>>
>> OK, thanks.  So the testcase never compiled on the trunk (except for about
>> 24
>> hours between 188009 & 188118) or did it compile before 188008 at some
>> point?
>>
>> --
>> Eric Botcazou
>
> It was OK until revision 187042:
>
>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>
>         * tree.c (valid_constant_size_p): New function.
>         * tree.h (valid_constant_size_p): Declare.
>         * cfgexpand.c (expand_one_var): Adjust check for too large
>         variables by using valid_constant_size_p.
>         * varasm.c (assemble_variable): Likewise.
>
>         c/
>         * c-decl.c (grokdeclarator): Properly check for sizes that
>         cover more than half of the address-space.
>
>         cp/
>         * decl.c (grokdeclarator): Properly check for sizes that
>         cover more than half of the address-space.
>
>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>
>         * fold-const.c (div_if_zero_remainder): sizetypes no longer
>         sign-extend.
>         (int_const_binop_1): New worker for int_const_binop with
>         overflowable parameter.  Pass it through
>         to force_fit_type_double.
>         (int_const_binop): Wrap around int_const_binop_1 with overflowable
>         equal to one.
>         (size_binop_loc): Call int_const_binop_1 with overflowable equal
>         to minus one, forcing overflow detection for even unsigned types.
>         (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
>         (fold_binary_loc): Call try_move_mult_to_index with signed offset.
>         * stor-layout.c (initialize_sizetypes): sizetypes no longer
>         sign-extend.
>         (layout_type): For zero-sized arrays ignore overflow on the
>         size calculations.
>         * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
>         (bit_value_binop_1): Likewise.
>         * tree.c (double_int_to_tree): Likewise.
>         (double_int_fits_to_tree_p): Likewise.
>         (force_fit_type_double): Likewise.
>         (host_integerp): Likewise.
>         (int_fits_type_p): Likewise.
>         * varasm.c (output_constructor_regular_field): Sign-extend the
>         field-offset to cater for negative offsets produced by the Ada frontend.
>         * omp-low.c (extract_omp_for_data): Convert the loop step to
>         signed for pointer adjustments.
>
> which gave:
>
> /export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O3 -Wall -mx32
> -maddress-mode=long -fPIC -S x.i
> x.i: In function ‘dl_start’:
> x.i:30:1: error: unrecognizable insn:
>  }
>  ^
> (insn 54 53 55 7 (set (reg:SI 108)
>         (const:SI (plus:SI (symbol_ref:SI ("_dl_rtld_map") [flags
> 0x42] <var_decl 0x7f02997bb140 _dl_rtld_map>)
>                 (const_int -1073742800 [0xffffffffbffffc30])))) x.i:22 -1
>      (nil))
> x.i:30:1: internal compiler error: in extract_insn, at recog.c:2130
>  }
>  ^
> Please submit a full bug report,
> with preprocessed source if appropriate.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>

It looks like we are using unsigned type for array index:

(gdb)  list
2163		tree element = TREE_TYPE (type);
2164	
2165		build_pointer_type (element);
2166	
2167		/* We need to know both bounds in order to compute the size.  */
2168		if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
2169		    && TYPE_SIZE (element))
2170		  {
2171		    tree ub = TYPE_MAX_VALUE (index);
2172		    tree lb = TYPE_MIN_VALUE (index);
(gdb) call debug_tree (index)
 <integer_type 0x7ffff1ab6000
    type <integer_type 0x7ffff199d000 sizetype public unsigned sizetype SI
        size <integer_cst 0x7ffff1989d40 constant 32>
        unit size <integer_cst 0x7ffff1989d60 constant 4>
        align 32 symtab 0 alias set -1 canonical type 0x7ffff199d000
precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
0x7ffff1989000 4294967295>>
    SI size <integer_cst 0x7ffff1989d40 32> unit size <integer_cst
0x7ffff1989d60 4>
    align 32 symtab 0 alias set -1 canonical type 0x7ffff1ab6000
precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
0x7ffff1aa3260 33>>
(gdb)


-- 
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-11-01  1:57                             ` H.J. Lu
@ 2012-11-01  2:14                               ` H.J. Lu
  2012-11-01  2:16                                 ` H.J. Lu
  2012-11-01  2:21                               ` Andrew Pinski
  1 sibling, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2012-11-01  2:14 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 6:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 3:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Oct 31, 2012 at 7:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>
>>> > It failed with revision 188008.
>>>
>>> OK, thanks.  So the testcase never compiled on the trunk (except for about
>>> 24
>>> hours between 188009 & 188118) or did it compile before 188008 at some
>>> point?
>>>
>>> --
>>> Eric Botcazou
>>
>> It was OK until revision 187042:
>>
>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>
>>         * tree.c (valid_constant_size_p): New function.
>>         * tree.h (valid_constant_size_p): Declare.
>>         * cfgexpand.c (expand_one_var): Adjust check for too large
>>         variables by using valid_constant_size_p.
>>         * varasm.c (assemble_variable): Likewise.
>>
>>         c/
>>         * c-decl.c (grokdeclarator): Properly check for sizes that
>>         cover more than half of the address-space.
>>
>>         cp/
>>         * decl.c (grokdeclarator): Properly check for sizes that
>>         cover more than half of the address-space.
>>
>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>
>>         * fold-const.c (div_if_zero_remainder): sizetypes no longer
>>         sign-extend.
>>         (int_const_binop_1): New worker for int_const_binop with
>>         overflowable parameter.  Pass it through
>>         to force_fit_type_double.
>>         (int_const_binop): Wrap around int_const_binop_1 with overflowable
>>         equal to one.
>>         (size_binop_loc): Call int_const_binop_1 with overflowable equal
>>         to minus one, forcing overflow detection for even unsigned types.
>>         (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
>>         (fold_binary_loc): Call try_move_mult_to_index with signed offset.
>>         * stor-layout.c (initialize_sizetypes): sizetypes no longer
>>         sign-extend.
>>         (layout_type): For zero-sized arrays ignore overflow on the
>>         size calculations.
>>         * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
>>         (bit_value_binop_1): Likewise.
>>         * tree.c (double_int_to_tree): Likewise.
>>         (double_int_fits_to_tree_p): Likewise.
>>         (force_fit_type_double): Likewise.
>>         (host_integerp): Likewise.
>>         (int_fits_type_p): Likewise.
>>         * varasm.c (output_constructor_regular_field): Sign-extend the
>>         field-offset to cater for negative offsets produced by the Ada frontend.
>>         * omp-low.c (extract_omp_for_data): Convert the loop step to
>>         signed for pointer adjustments.
>>
>> which gave:
>>
>> /export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
>> -B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O3 -Wall -mx32
>> -maddress-mode=long -fPIC -S x.i
>> x.i: In function ‘dl_start’:
>> x.i:30:1: error: unrecognizable insn:
>>  }
>>  ^
>> (insn 54 53 55 7 (set (reg:SI 108)
>>         (const:SI (plus:SI (symbol_ref:SI ("_dl_rtld_map") [flags
>> 0x42] <var_decl 0x7f02997bb140 _dl_rtld_map>)
>>                 (const_int -1073742800 [0xffffffffbffffc30])))) x.i:22 -1
>>      (nil))
>> x.i:30:1: internal compiler error: in extract_insn, at recog.c:2130
>>  }
>>  ^
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>
>
> It looks like we are using unsigned type for array index:
>
> (gdb)  list
> 2163            tree element = TREE_TYPE (type);
> 2164
> 2165            build_pointer_type (element);
> 2166
> 2167            /* We need to know both bounds in order to compute the size.  */
> 2168            if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
> 2169                && TYPE_SIZE (element))
> 2170              {
> 2171                tree ub = TYPE_MAX_VALUE (index);
> 2172                tree lb = TYPE_MIN_VALUE (index);
> (gdb) call debug_tree (index)
>  <integer_type 0x7ffff1ab6000
>     type <integer_type 0x7ffff199d000 sizetype public unsigned sizetype SI
>         size <integer_cst 0x7ffff1989d40 constant 32>
>         unit size <integer_cst 0x7ffff1989d60 constant 4>
>         align 32 symtab 0 alias set -1 canonical type 0x7ffff199d000
> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
> 0x7ffff1989000 4294967295>>
>     SI size <integer_cst 0x7ffff1989d40 32> unit size <integer_cst
> 0x7ffff1989d60 4>
>     align 32 symtab 0 alias set -1 canonical type 0x7ffff1ab6000
> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
> 0x7ffff1aa3260 33>>
> (gdb)
>

size type is changed to unsigned:

-  /* Size types *are* sign extended.  */
-  bool sign_extended_type = (!TYPE_UNSIGNED (type)
-			     || (TREE_CODE (type) == INTEGER_TYPE
-				 && TYPE_IS_SIZETYPE (type)));
+  bool sign_extended_type = !TYPE_UNSIGNED (type);

But we use size type in places where signed size type
should be used.

-- 
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-11-01  2:14                               ` H.J. Lu
@ 2012-11-01  2:16                                 ` H.J. Lu
  0 siblings, 0 replies; 21+ messages in thread
From: H.J. Lu @ 2012-11-01  2:16 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 7:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 6:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Oct 31, 2012 at 3:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Oct 31, 2012 at 7:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>>
>>>> > It failed with revision 188008.
>>>>
>>>> OK, thanks.  So the testcase never compiled on the trunk (except for about
>>>> 24
>>>> hours between 188009 & 188118) or did it compile before 188008 at some
>>>> point?
>>>>
>>>> --
>>>> Eric Botcazou
>>>
>>> It was OK until revision 187042:
>>>
>>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>>
>>>         * tree.c (valid_constant_size_p): New function.
>>>         * tree.h (valid_constant_size_p): Declare.
>>>         * cfgexpand.c (expand_one_var): Adjust check for too large
>>>         variables by using valid_constant_size_p.
>>>         * varasm.c (assemble_variable): Likewise.
>>>
>>>         c/
>>>         * c-decl.c (grokdeclarator): Properly check for sizes that
>>>         cover more than half of the address-space.
>>>
>>>         cp/
>>>         * decl.c (grokdeclarator): Properly check for sizes that
>>>         cover more than half of the address-space.
>>>
>>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>>
>>>         * fold-const.c (div_if_zero_remainder): sizetypes no longer
>>>         sign-extend.
>>>         (int_const_binop_1): New worker for int_const_binop with
>>>         overflowable parameter.  Pass it through
>>>         to force_fit_type_double.
>>>         (int_const_binop): Wrap around int_const_binop_1 with overflowable
>>>         equal to one.
>>>         (size_binop_loc): Call int_const_binop_1 with overflowable equal
>>>         to minus one, forcing overflow detection for even unsigned types.
>>>         (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
>>>         (fold_binary_loc): Call try_move_mult_to_index with signed offset.
>>>         * stor-layout.c (initialize_sizetypes): sizetypes no longer
>>>         sign-extend.
>>>         (layout_type): For zero-sized arrays ignore overflow on the
>>>         size calculations.
>>>         * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
>>>         (bit_value_binop_1): Likewise.
>>>         * tree.c (double_int_to_tree): Likewise.
>>>         (double_int_fits_to_tree_p): Likewise.
>>>         (force_fit_type_double): Likewise.
>>>         (host_integerp): Likewise.
>>>         (int_fits_type_p): Likewise.
>>>         * varasm.c (output_constructor_regular_field): Sign-extend the
>>>         field-offset to cater for negative offsets produced by the Ada frontend.
>>>         * omp-low.c (extract_omp_for_data): Convert the loop step to
>>>         signed for pointer adjustments.
>>>
>>> which gave:
>>>
>>> /export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
>>> -B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O3 -Wall -mx32
>>> -maddress-mode=long -fPIC -S x.i
>>> x.i: In function ‘dl_start’:
>>> x.i:30:1: error: unrecognizable insn:
>>>  }
>>>  ^
>>> (insn 54 53 55 7 (set (reg:SI 108)
>>>         (const:SI (plus:SI (symbol_ref:SI ("_dl_rtld_map") [flags
>>> 0x42] <var_decl 0x7f02997bb140 _dl_rtld_map>)
>>>                 (const_int -1073742800 [0xffffffffbffffc30])))) x.i:22 -1
>>>      (nil))
>>> x.i:30:1: internal compiler error: in extract_insn, at recog.c:2130
>>>  }
>>>  ^
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>>
>>
>> It looks like we are using unsigned type for array index:
>>
>> (gdb)  list
>> 2163            tree element = TREE_TYPE (type);
>> 2164
>> 2165            build_pointer_type (element);
>> 2166
>> 2167            /* We need to know both bounds in order to compute the size.  */
>> 2168            if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
>> 2169                && TYPE_SIZE (element))
>> 2170              {
>> 2171                tree ub = TYPE_MAX_VALUE (index);
>> 2172                tree lb = TYPE_MIN_VALUE (index);
>> (gdb) call debug_tree (index)
>>  <integer_type 0x7ffff1ab6000
>>     type <integer_type 0x7ffff199d000 sizetype public unsigned sizetype SI
>>         size <integer_cst 0x7ffff1989d40 constant 32>
>>         unit size <integer_cst 0x7ffff1989d60 constant 4>
>>         align 32 symtab 0 alias set -1 canonical type 0x7ffff199d000
>> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
>> 0x7ffff1989000 4294967295>>
>>     SI size <integer_cst 0x7ffff1989d40 32> unit size <integer_cst
>> 0x7ffff1989d60 4>
>>     align 32 symtab 0 alias set -1 canonical type 0x7ffff1ab6000
>> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
>> 0x7ffff1aa3260 33>>
>> (gdb)
>>
>
> size type is changed to unsigned:
>
> -  /* Size types *are* sign extended.  */
> -  bool sign_extended_type = (!TYPE_UNSIGNED (type)
> -                            || (TREE_CODE (type) == INTEGER_TYPE
> -                                && TYPE_IS_SIZETYPE (type)));
> +  bool sign_extended_type = !TYPE_UNSIGNED (type);
>
> But we use size type in places where signed size type
> should be used.
>

For example, array index computation must be signed,
not unsigned.


-- 
H.J.

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

* Re: [PATCH] Fix PR53501
  2012-11-01  1:57                             ` H.J. Lu
  2012-11-01  2:14                               ` H.J. Lu
@ 2012-11-01  2:21                               ` Andrew Pinski
  1 sibling, 0 replies; 21+ messages in thread
From: Andrew Pinski @ 2012-11-01  2:21 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Eric Botcazou, gcc-patches, Richard Guenther

On Wed, Oct 31, 2012 at 6:57 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Oct 31, 2012 at 3:53 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Oct 31, 2012 at 7:29 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>>
>>> > It failed with revision 188008.
>>>
>>> OK, thanks.  So the testcase never compiled on the trunk (except for about
>>> 24
>>> hours between 188009 & 188118) or did it compile before 188008 at some
>>> point?
>>>
>>> --
>>> Eric Botcazo>>
>> It was OK until revision 187042:
>>
>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>
>>         * tree.c (valid_constant_size_p): New function.
>>         * tree.h (valid_constant_size_p): Declare.
>>         * cfgexpand.c (expand_one_var): Adjust check for too large
>>         variables by using valid_constant_size_p.
>>         * varasm.c (assemble_variable): Likewise.
>>
>>         c/
>>         * c-decl.c (grokdeclarator): Properly check for sizes that
>>         cover more than half of the address-space.
>>
>>         cp/
>>         * decl.c (grokdeclarator): Properly check for sizes that
>>         cover more than half of the address-space.
>>
>>     2012-05-02  Richard Guenther  <rguenther@suse.de>
>>
>>         * fold-const.c (div_if_zero_remainder): sizetypes no longer
>>         sign-extend.
>>         (int_const_binop_1): New worker for int_const_binop with
>>         overflowable parameter.  Pass it through
>>         to force_fit_type_double.
>>         (int_const_binop): Wrap around int_const_binop_1 with overflowable
>>         equal to one.
>>         (size_binop_loc): Call int_const_binop_1 with overflowable equal
>>         to minus one, forcing overflow detection for even unsigned types.
>>         (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
>>         (fold_binary_loc): Call try_move_mult_to_index with signed offset.
>>         * stor-layout.c (initialize_sizetypes): sizetypes no longer
>>         sign-extend.
>>         (layout_type): For zero-sized arrays ignore overflow on the
>>         size calculations.
>>         * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
>>         (bit_value_binop_1): Likewise.
>>         * tree.c (double_int_to_tree): Likewise.
>>         (double_int_fits_to_tree_p): Likewise.
>>         (force_fit_type_double): Likewise.
>>         (host_integerp): Likewise.
>>         (int_fits_type_p): Likewise.
>>         * varasm.c (output_constructor_regular_field): Sign-extend the
>>         field-offset to cater for negative offsets produced by the Ada frontend.
>>         * omp-low.c (extract_omp_for_data): Convert the loop step to
>>         signed for pointer adjustments.
>>
>> which gave:
>>
>> /export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
>> -B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -O3 -Wall -mx32
>> -maddress-mode=long -fPIC -S x.i
>> x.i: In function ‘dl_start’:
>> x.i:30:1: error: unrecognizable insn:
>>  }
>>  ^
>> (insn 54 53 55 7 (set (reg:SI 108)
>>         (const:SI (plus:SI (symbol_ref:SI ("_dl_rtld_map") [flags
>> 0x42] <var_decl 0x7f02997bb140 _dl_rtld_map>)
>>                 (const_int -1073742800 [0xffffffffbffffc30])))) x.i:22 -1
>>      (nil))
>> x.i:30:1: internal compiler error: in extract_insn, at recog.c:2130
>>  }
>>  ^
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>
>
> It looks like we are using unsigned type for array index:

We always used unsigned type for array index and for the left hand
side of POINTER_PLUS_EXPR.  I think your issue is somewhere else.
Like maybe in expand.

Thanks,
Andrew Pinski


>
> (gdb)  list
> 2163            tree element = TREE_TYPE (type);
> 2164
> 2165            build_pointer_type (element);
> 2166
> 2167            /* We need to know both bounds in order to compute the size.  */
> 2168            if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
> 2169                && TYPE_SIZE (element))
> 2170              {
> 2171                tree ub = TYPE_MAX_VALUE (index);
> 2172                tree lb = TYPE_MIN_VALUE (index);
> (gdb) call debug_tree (index)
>  <integer_type 0x7ffff1ab6000
>     type <integer_type 0x7ffff199d000 sizetype public unsigned sizetype SI
>         size <integer_cst 0x7ffff1989d40 constant 32>
>         unit size <integer_cst 0x7ffff1989d60 constant 4>
>         align 32 symtab 0 alias set -1 canonical type 0x7ffff199d000
> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
> 0x7ffff1989000 4294967295>>
>     SI size <integer_cst 0x7ffff1989d40 32> unit size <integer_cst
> 0x7ffff1989d60 4>
>     align 32 symtab 0 alias set -1 canonical type 0x7ffff1ab6000
> precision 32 min <integer_cst 0x7ffff1989d80 0> max <integer_cst
> 0x7ffff1aa3260 33>>
> (gdb)
>
>
> --
> H.J.

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

* Re: [PATCH] Fix PR53501
  2012-10-31 23:03                           ` H.J. Lu
  2012-11-01  1:57                             ` H.J. Lu
@ 2012-11-01  8:43                             ` Eric Botcazou
  1 sibling, 0 replies; 21+ messages in thread
From: Eric Botcazou @ 2012-11-01  8:43 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Richard Guenther

> It was OK until revision 187042:
> 
>     2012-05-02  Richard Guenther  <rguenther@suse.de>
> 
>         * tree.c (valid_constant_size_p): New function.
>         * tree.h (valid_constant_size_p): Declare.
>         * cfgexpand.c (expand_one_var): Adjust check for too large
>         variables by using valid_constant_size_p.
>         * varasm.c (assemble_variable): Likewise.
> 
>         c/
>         * c-decl.c (grokdeclarator): Properly check for sizes that
>         cover more than half of the address-space.
> 
>         cp/
>         * decl.c (grokdeclarator): Properly check for sizes that
>         cover more than half of the address-space.
> 
>     2012-05-02  Richard Guenther  <rguenther@suse.de>
> 
>         * fold-const.c (div_if_zero_remainder): sizetypes no longer
>         sign-extend.
>         (int_const_binop_1): New worker for int_const_binop with
>         overflowable parameter.  Pass it through
>         to force_fit_type_double.
>         (int_const_binop): Wrap around int_const_binop_1 with overflowable
>         equal to one.
>         (size_binop_loc): Call int_const_binop_1 with overflowable equal
>         to minus one, forcing overflow detection for even unsigned types.
>         (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing.
>         (fold_binary_loc): Call try_move_mult_to_index with signed offset.
>         * stor-layout.c (initialize_sizetypes): sizetypes no longer
>         sign-extend.
>         (layout_type): For zero-sized arrays ignore overflow on the
>         size calculations.
>         * tree-ssa-ccp.c (bit_value_unop_1): Likewise.
>         (bit_value_binop_1): Likewise.
>         * tree.c (double_int_to_tree): Likewise.
>         (double_int_fits_to_tree_p): Likewise.
>         (force_fit_type_double): Likewise.
>         (host_integerp): Likewise.
>         (int_fits_type_p): Likewise.
>         * varasm.c (output_constructor_regular_field): Sign-extend the field
>         offset to cater for negative offsets produced by the Ada frontend.
>	    * omp-low.c (extract_omp_for_data): Convert the loop step to
>         signed for pointer adjustments.

OK, thanks for digging.  I'll have a look.

-- 
Eric Botcazou

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

end of thread, other threads:[~2012-11-01  8:43 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-30 10:42 [PATCH] Fix PR53501 Richard Guenther
2012-05-31 12:31 ` Eric Botcazou
2012-05-31 12:38   ` Richard Guenther
2012-06-01  8:49     ` Eric Botcazou
2012-06-01  8:52       ` Richard Guenther
2012-06-01  9:40         ` Eric Botcazou
2012-06-01  9:44           ` Richard Guenther
2012-06-01  9:54             ` Eric Botcazou
2012-06-01 10:09               ` Richard Guenther
2012-06-01 10:31                 ` Eric Botcazou
2012-06-01 10:34                   ` Richard Guenther
2012-10-31 10:30                   ` H.J. Lu
2012-10-31 12:49                     ` Eric Botcazou
2012-10-31 13:06                       ` H.J. Lu
2012-10-31 14:41                         ` Eric Botcazou
2012-10-31 23:03                           ` H.J. Lu
2012-11-01  1:57                             ` H.J. Lu
2012-11-01  2:14                               ` H.J. Lu
2012-11-01  2:16                                 ` H.J. Lu
2012-11-01  2:21                               ` Andrew Pinski
2012-11-01  8:43                             ` Eric Botcazou

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).