diff mbox

Make sizetypes no longer sign-extending

Message ID Pine.LNX.4.64.1204251539170.23071@jbgna.fhfr.qr
State New
Headers show

Commit Message

Richard Biener April 25, 2012, 1:49 p.m. UTC 
On Tue, 24 Apr 2012, Richard Guenther wrote:

> On Tue, 24 Apr 2012, Richard Guenther wrote:
> 
> > 
> > I've been carrying this patch for quite some while now and really
> > want to go forward with it - the problem is that while all default
> > languages work fine after this patch Ada shows some testsuite
> > regressions.  I've had various hacks/workarounds throughout the
> > Ada frontend for them, but lost track of what fixed what and
> > they all felt like hacks anyway.
> > 
> > Thus - I know the patch will add Ada testsuite regressions.  But it will
> > not break Ada bootstrap.  Ada is not in the set of default languages,
> > nor is it considered release critical.
> > 
> > Are the Ada folks happy with helping to fix the fallout after-the-fact
> > (I got Eric to fix the bootstrap issues that were first present - thanks
> > for that)?  I am happy to revisit my hacks/workarounds and post them,
> > but it will be ultimatively easier to review them if you can see
> > the FAIL for yourself (there are some workarounds/hacks posted on the
> > mailinglist for previous attempts IIRC).
> > 
> > Thanks for your consideration.
> > 
> > The patch is currently under re-testing (it needs the 2nd patch
> > below, which was already approved but back in time broke Ada
> > bootstrap - I didn't verify if that still occurs).
> 
> To followup myself - bootstrap with just the 2nd patch is still
> broken:
> 
> /abuild/rguenther/obj2/./gcc/xgcc -B/abuild/rguenther/obj2/./gcc/ 
> -B/usr/local/x86_64-unknown-linux-gnu/bin/ 
> -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem 
> /usr/local/x86_64-unknown-linux-gnu/include -isystem 
> /usr/local/x86_64-unknown-linux-gnu/sys-include    -c -g -O2 -m32 -fpic  
> -W -Wall -gnatpg -nostdinc -m32  s-secsta.adb -o s-secsta.o
> s-secsta.adb:501:4: error: size of variable 'System.Secondary_Stack.Chunk' 
> is too large
>     Chunk : aliased Chunk_Id (1, Static_Secondary_Stack_Size);
>     ^
> make[9]: *** [s-secsta.o] Error 1
> 
> And the following is the list of regressions introduced by the combined
> patch set:
> 
>                 === acats tests ===
> FAIL:   a71004a
> FAIL:   c36204d
> FAIL:   c36205l
> FAIL:   c37404b
> FAIL:   c41107a
> FAIL:   c41204a
> FAIL:   c43204c
> FAIL:   c43204e
> FAIL:   c43204f
> FAIL:   c43204g
> FAIL:   c43204h
> FAIL:   c43204i
> FAIL:   c52102a
> FAIL:   c52102c
> FAIL:   c64103c
> FAIL:   c64103d
> FAIL:   c64106a
> FAIL:   c95087a
> FAIL:   cc1224a
> FAIL:   cc1311a
> FAIL:   cc3106b
> FAIL:   cc3224a
> FAIL:   cd2a31a
> 
>                 === acats Summary ===
> # of expected passes            2297
> # of unexpected failures        23
> 
>                 === gnat tests ===
> 
> 
> Running target unix/
> FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
> FAIL: gnat.dg/loop_optimization3.adb execution test
> FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
> FAIL: gnat.dg/test_8bitlong_overflow.adb (test for excess errors)
> FAIL: gnat.dg/test_8bitlong_overflow.adb execution test
> 
>                 === gnat Summary for unix/ ===
> 
> # of expected passes            1089
> # of unexpected failures        8
> # of expected failures          13
> # of unsupported tests          2
> 
> Running target unix//-m32
> FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
> FAIL: gnat.dg/loop_optimization3.adb execution test
> FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
> FAIL: gnat.dg/test_8bitlong_overflow.adb (test for excess errors)
> FAIL: gnat.dg/test_8bitlong_overflow.adb execution test
> 
>                 === gnat Summary for unix//-m32 ===
> 
> # of expected passes            1089
> # of unexpected failures        8
> # of expected failures          13
> # of unsupported tests          2
> 
>                 === gnat Summary ===
> 
> # of expected passes            2178
> # of unexpected failures        16
> # of expected failures          26
> # of unsupported tests          4
> 
> Most of the ACATS errors are "raised STORAGE_ERROR : object too large"
> or "error: size of variable '...' is too large".  The gnat testsuite
> adds "warning: "Storage_Error" will be raised at run time" to this.
> 
> I remember one workaround (which usually involves re-setting TREE_OVERFLOW
> at strathegic places) fixes most of ACATS.  I'll try to isolate that
> (but it's a hack and does not feel "right").

Ah, and all ACATS fails and

-FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
-FAIL: gnat.dg/loop_optimization3.adb execution test
-FAIL: gnat.dg/test_8bitlong_overflow.adb (test for excess errors)
-FAIL: gnat.dg/test_8bitlong_overflow.adb execution test

are fixed by for example


thus are because array TYPE_DOMAIN is built using unsigned sizetype
but these Ada testcases have array domains which really need signed
types.  The above is of course a hack, but one that otherwise survives
bootstrap / test of all languages.

Thus, we arrive at the following Ada regression status if the patch series
is applied (plus the above incremental patch):

                === acats tests ===

                === acats Summary ===
# of expected passes            2320
# of unexpected failures        0
Native configuration is x86_64-unknown-linux-gnu

                === gnat tests ===


Running target unix/
FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
FAIL: gnat.dg/return3.adb scan-assembler loc 1 6

                === gnat Summary for unix/ ===

# of expected passes            1093
# of unexpected failures        4
# of expected failures          13
# of unsupported tests          2

Running target unix//-m32
FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
FAIL: gnat.dg/return3.adb scan-assembler loc 1 6

                === gnat Summary for unix//-m32 ===

# of expected passes            1093
# of unexpected failures        4
# of expected failures          13
# of unsupported tests          2

                === gnat Summary ===

# of expected passes            2186
# of unexpected failures        8
# of expected failures          26
# of unsupported tests          4


Which I consider reasonable?

Thanks,
Richard.

Comments

Eric Botcazou April 27, 2012, 10:13 a.m. UTC | #1 
> Ah, and all ACATS fails and
>
> -FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
> -FAIL: gnat.dg/loop_optimization3.adb execution test
> -FAIL: gnat.dg/test_8bitlong_overflow.adb (test for excess errors)
> -FAIL: gnat.dg/test_8bitlong_overflow.adb execution test
>
> are fixed by for example
>
> [...]
>
> thus are because array TYPE_DOMAIN is built using unsigned sizetype
> but these Ada testcases have array domains which really need signed
> types.  The above is of course a hack, but one that otherwise survives
> bootstrap / test of all languages.

Kind of a miracle if you ask me, but probably a reasonable way out for Ada.
Thanks a lot for devising it.

> Thus, we arrive at the following Ada regression status if the patch series
> is applied (plus the above incremental patch):
>
>                 === acats tests ===
>
>                 === acats Summary ===
> # of expected passes            2320
> # of unexpected failures        0
> Native configuration is x86_64-unknown-linux-gnu
>
>                 === gnat tests ===
>
>
> Running target unix/
> FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
>
>                 === gnat Summary for unix/ ===
>
> # of expected passes            1093
> # of unexpected failures        4
> # of expected failures          13
> # of unsupported tests          2
>
> Running target unix//-m32
> FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
>
>                 === gnat Summary for unix//-m32 ===
>
> # of expected passes            1093
> # of unexpected failures        4
> # of expected failures          13
> # of unsupported tests          2
>
>                 === gnat Summary ===
>
> # of expected passes            2186
> # of unexpected failures        8
> # of expected failures          26
> # of unsupported tests          4
>
>
> Which I consider reasonable?

Sure, no opposition by me to applying the whole set of patches.
Richard Biener May 2, 2012, 11:22 a.m. UTC | #2 
On Fri, 27 Apr 2012, Eric Botcazou wrote:

> > Ah, and all ACATS fails and
> >
> > -FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
> > -FAIL: gnat.dg/loop_optimization3.adb execution test
> > -FAIL: gnat.dg/test_8bitlong_overflow.adb (test for excess errors)
> > -FAIL: gnat.dg/test_8bitlong_overflow.adb execution test
> >
> > are fixed by for example
> >
> > [...]
> >
> > thus are because array TYPE_DOMAIN is built using unsigned sizetype
> > but these Ada testcases have array domains which really need signed
> > types.  The above is of course a hack, but one that otherwise survives
> > bootstrap / test of all languages.
> 
> Kind of a miracle if you ask me, but probably a reasonable way out for Ada.
> Thanks a lot for devising it.
> 
> > Thus, we arrive at the following Ada regression status if the patch series
> > is applied (plus the above incremental patch):
> >
> >                 === acats tests ===
> >
> >                 === acats Summary ===
> > # of expected passes            2320
> > # of unexpected failures        0
> > Native configuration is x86_64-unknown-linux-gnu
> >
> >                 === gnat tests ===
> >
> >
> > Running target unix/
> > FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> > FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> > FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> > FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
> >
> >                 === gnat Summary for unix/ ===
> >
> > # of expected passes            1093
> > # of unexpected failures        4
> > # of expected failures          13
> > # of unsupported tests          2
> >
> > Running target unix//-m32
> > FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
> > FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
> > FAIL: gnat.dg/renaming5.adb scan-tree-dump-times optimized "goto" 2
> > FAIL: gnat.dg/return3.adb scan-assembler loc 1 6
> >
> >                 === gnat Summary for unix//-m32 ===
> >
> > # of expected passes            1093
> > # of unexpected failures        4
> > # of expected failures          13
> > # of unsupported tests          2
> >
> >                 === gnat Summary ===
> >
> > # of expected passes            2186
> > # of unexpected failures        8
> > # of expected failures          26
> > # of unsupported tests          4
> >
> >
> > Which I consider reasonable?
> 
> Sure, no opposition by me to applying the whole set of patches.

Done now.

Thanks,
Richard.
diff mbox

Patch

Index: trunk/gcc/stor-layout.c
===================================================================
--- trunk.orig/gcc/stor-layout.c	2012-04-25 14:14:52.321710059 +0200
+++ trunk/gcc/stor-layout.c	2012-04-25 14:13:43.595714163 +0200
@@ -2182,11 +2182,29 @@  layout_type (tree type)
 	       that (possible) negative values are handled appropriately
 	       when determining overflow.  */
 	    else
-	      length
-		= fold_convert (sizetype,
-				size_binop (PLUS_EXPR,
-					    build_int_cst (TREE_TYPE (lb), 1),
-					    size_binop (MINUS_EXPR, ub, lb)));
+	      {
+		/* ???  When it is obvious that the range is signed
+		   represent it using ssizetype.  */
+		if (TREE_CODE (lb) == INTEGER_CST
+		    && TREE_CODE (ub) == INTEGER_CST
+		    && TYPE_UNSIGNED (TREE_TYPE (lb))
+		    && tree_int_cst_lt (ub, lb))
+		  {
+		    lb = double_int_to_tree
+			   (ssizetype,
+			    double_int_sext (tree_to_double_int (lb),
+					     TYPE_PRECISION (TREE_TYPE (lb))));
+		    ub = double_int_to_tree
+			   (ssizetype,
+			    double_int_sext (tree_to_double_int (ub),
+					     TYPE_PRECISION (TREE_TYPE (ub))));
+		  }
+		length
+		  = fold_convert (sizetype,
+				  size_binop (PLUS_EXPR,
+					      build_int_cst (TREE_TYPE (lb), 1),
+					      size_binop (MINUS_EXPR, ub, lb)));
+	      }
 
 	    /* If we arrived at a length of zero ignore any overflow
 	       that occured as part of the calculation.  There exists